Re: [Xen-devel] [PATCH V4 22/24] xl: update domain configuration when we hotplug a device

[Ian Campbell <Ian.Campbell@xxxxxxxxxx>]

On Tue, 2014-05-06 at 16:58 +0100, Wei Liu wrote:
> > > +    libxl_device_pci pcidev, *ppcidev;
> > 
> > Likewise ppcidev here, perhaps meaning you would need to pass the type
> > rather then an instance to the variable.
> > 
> > >      XLU_Config *config;
> > >  
> > >      libxl_device_pci_init(&pcidev);
> > > @@ -3059,7 +3090,12 @@ static void pciattach(uint32_t domid, const char 
> > > *bdf, const char *vs)
> > >          fprintf(stderr, "pci-attach: malformed BDF specification 
> > > \"%s\"\n", bdf);
> > >          exit(2);
> > >      }
> > > -    libxl_device_pci_add(ctx, domid, &pcidev, 0);
> > > +
> > > +    ADD_DEVICE(pci, domid, pcidev, &d_config, ppcidev, {
> > > +            ppcidev = ARRAY_EXTEND_INIT_NODEVID(d_config.pcidevs,
> > > +                                                d_config.num_pcidevs,
> > > +                                                libxl_device_pci_init);
> > 
> > Do you really need to do a deep copy of pcidev into the new array slot?
> > If you just memcpy the thing over and then do not dispose the original
> > then the copy will simply take over those allocations, which is fine I
> > think.
> > 
> 
> The copy is done so that we know what user provides at the beginning,
> i.e. this is the template that we need to keep.  After the structure is
> handed back by libxl the content might be altered already, say, all
> "default" values replaced by libxl with some values that it deems
> sensible. Then fixup will run to copy those bits we care about back to
> the template.

I see. (A shame, avoiding this deep copy stuff would have simplified all
sorts of things!)

BTW I think it's not so much the replacement of fields but the freeing
of the old values which prevents the shallow copy.

> > > +            libxl_mac_copy(ctx, &pnic->mac, &nic.mac);
> > 
> > Doesn't the copy handle this already? If not then why not?
> > 
> 
> This is the fixup. If user doesn't supply uuid, libxl generates one for
> this device and we really want to save it in our state. If user supplies
> before hand, it just copies the original value back.
> 
> The fixup is device dependent. You may find nic has different fixup and
> disk has no fixup at all.

I see. eventually we might want to provide this fixup as a libxl helper
function, but having it here for now will let us bed it in first.

Ian.


_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel