@jstarks After rework galore 😄

There was some recent changes that after rebasing were a pain to handle. Most of the annoyances are in wcow lol. Edit: Ok after consulting with the team, seems like we're all fine with axing the internal connection completely. huzzah, things get prettier. #1038 The main pain point with this is that we still "support" the internal gc connection (the HCS managed one). Right now this won't work at all if you wanted to use the internal guest conn

Here's some of the ugly that if we can agree that we don't see a need to keep around the internal conn can get rid of:
1. https://github.com/microsoft/hcsshim/pull/1037/files#diff-897d0148db083dffc20d984ea4d9aa1dc4f2dd4b36b1bf7c61952e6cb51f5da6R296-R309
For this it was just some branching logic to grab the gcs capabilities through HCS itself, not sure how that would fit into the UVM interface.

2. Then launching a container in the UVM: https://github.com/microsoft/hcsshim/pull/1037/files#diff-897d0148db083dffc20d984ea4d9aa1dc4f2dd4b36b1bf7c61952e6cb51f5da6L270-R308
Gross. There's probably a better way to handle that but I left it in just to show what can be avoided if we can just always assume we have a conn in-proc.

I think we kept the fallback to the internal conn originally in case there was any issues when swapping to external, but I don't ~~~~think we've seen any issues since the swap so.. 😬

Here's the biggest thing I want feedback on: https://github.com/microsoft/hcsshim/pull/1037/files#diff-25e9fbfbd912fbf50e566d845edbc79a704c57df49456d8c1c6e2e6c011d8506R8

For the builder (I don't think the same pattern can work for runtime adds) what do you think about building up the document using this pattern of WithX where the WithX functions return a vm.CreateOpt that gets applied right before creating the VM? https://github.com/microsoft/hcsshim/pull/1037/files#diff-8cecd4fadb9d3373bac162470231b5fdb41aa510959d0ee30089e0f797b81cc8R77-R82

I'd originally made the CreateOpt type to handle all of the weird "what would this even look like on the UVM interface" HCS stuff like registry additions, some extra things you need to set if the VM is going to be saved as a template etc. but honestly this pattern might be better suited to create the entire document.

For instance, instead of:

 if uvm.scsiControllerCount > 0 {
	scsi, ok := uvmb.(vm.SCSIManager)
	if !ok {
		return nil, errors.Wrap(vm.ErrNotSupported, "stopping SCSI setup")
	}
	for i := 0; i < int(uvm.scsiControllerCount); i++ {
		if err := scsi.AddSCSIController(uint32(i)); err != nil {
			return nil, errors.Wrap(err, "failed to add scsi controller")
		}
	}
}

if uvm.vpmemMaxCount > 0 {
	vpmem, ok := uvmb.(vm.VPMemManager)
	if !ok {
		return nil, errors.Wrap(vm.ErrNotSupported, "stopping VPMem setup")
	}
	if err := vpmem.AddVPMemController(uvm.vpmemMaxCount, uvm.vpmemMaxSizeBytes); err != nil {
		return nil, errors.Wrap(err, "failed to add VPMem controller")
	}
}

you can have:

if uvm.scsiControllerCount > 0 {
	for i := 0; i < int(uvm.scsiControllerCount); i++ {
		cOpts = append(cOpts, vm.WithSCSIController(uint32(i)))
	}
}

if uvm.vpmemMaxCount > 0 {
	cOpts = append(cOpts, vm.WithVPMemController(uvm.vpmemMaxCount, uvm.vpmemMaxSizeBytes))
}

and the With functions can do the typecasting inside and just call the corresponding method

func WithSerialConsole(port uint32, listenerPath string) CreateOpt {
	return func(ctx context.Context, uvmb UVMBuilder) error {
		serial, ok := uvmb.(SerialManager)
		if !ok {
			return vm.ErrNotSupported
		}
		return serial.SetSerialConsole(port, listenerPath)
	}
}

Just going to draft this, this is so out of date and has fallen off the priority list.

Closing to declutter the PR list view. Will revisit when it's back on the priority list