Re: [Xen-devel] [PATCH v1 05/10] libxl: synchronise configuration when we hotplug a device

[Ian Jackson <Ian.Jackson@xxxxxxxxxxxxx>]

Wei Liu writes ("Re: [PATCH v1 05/10] libxl: synchronise configuration when we 
hotplug a device"):
> On Fri, Jul 25, 2014 at 05:06:11PM +0100, Ian Jackson wrote:
> [...]
> > I don't think this is right.  At this point it may be the case that
> > the disk is already attached.
> 
> What do you mean by "attched"? I don't think guest knows anything about
> that disk at this point.

I mean, suppose I do this
     xl block-attach /dev/vm/foo,,xvda
     xl block-attach /dev/vm/bar,,xvda
then your code running during the second attach will remove
foo,,xvda from the json config while it remains in the domain.  That's
wrong.

> I'm a bit confused. If we ever come back to the beginning of the loop,
> doesn't that mean this xenstore transaction is not committed, so that
> any change to xenstore entries is not visible to others? That is,
> libxl__device_generic_add is doing the right thing (at least in this
> loop), as well as my call to DEVICE_REMOVE_JSON.

I meant, a device which existed previously.

> > > +#define LOCK_DOMAIN_JSON_CONFIG(gc, domid, lock, rc)                 \
> > > +    do {                                                             \
> > > +        if (domid != LIBXL_TOOLSTACK_DOMID)                          \
> > > +            rc = libxl__lock_domain_configuration(gc, domid, &lock); \
> > > +        else                                                         \
> > > +            rc = 0;                                                  \
> > > +    } while (0)
> > 
> > Can this test be perhaps moved into libxl__lock_domain_configuration ?
> 
> Sure, but Ian C suggested I generate a stub JSON config for Dom0 so that
> we can get rid of this special case. I think I will go down that route.

That would be even better.

> > I considered whether it would be possible to make this macro a
> > function instead but I think it's quite tricky because it would
> > involve a lot of casting.  (Eg of a function pointer type.)  I haven't
> > looked up the exact rules to see whether the resulting code would be
> > legal C99 but I think it would probably be clumsier so we should
> > probably stick with the macro.
> > 
> 
> Yes I would rather stick with this macro.

Right.

Thanks,
Ian.

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