Re: [Xen-devel] [PATCH v1 07/10] libxl: make libxl_cd_insert "eject" + "insert"

[Wei Liu <wei.liu2@xxxxxxxxxx>]

On Thu, Jul 17, 2014 at 11:44:35AM +0100, Ian Campbell wrote:
[...]
> >  
> > +/*
> > + * A "cdrom insert" is always processed as "eject" + "insert", with
> > + * updating JSON in between. So that we can know the current state of
> > + * CDROM later when we try to retrieve domain configuration: if
> > + * xenstore is "empty", then CDROM is "empty"; otherwise use the image
> > + * in JSON.
> 
> I think you could short circuit the case where the user inserts an empty
> disk (AKA ejects), right now you will eject twice. (We don't have an
> explicit libxl_cdrom_eject so I presume this is how it is intended to be
> done).
> 

OK. This is a valid optimisation.

> I think this function also handles HVM emulated IDE CDROM, which are
> semantically a bit different from PV CDROMs in that the empty drive is
> still an actual thing. Not sure if/how this impacts on your handling of
> the JSON though.
> 

IMO it wouldn't impact that much. I don't actually care what type of
backend it is using. I only care about whether the operation succeeds or
not.

> > diff --git a/tools/libxl/libxl_internal.h b/tools/libxl/libxl_internal.h
> > index b4f518a..f8f2ba2 100644
> > --- a/tools/libxl/libxl_internal.h
> > +++ b/tools/libxl/libxl_internal.h
> > @@ -3355,6 +3355,40 @@ static inline void 
> > libxl__update_config_vtpm(libxl__gc *gc,
> >          libxl_domain_config_dispose(&d_config);                         \
> >      } while (0)
> >  
> > +/* Search device list for device with the same identifier as "dev",
> > + * update that device with the content pointed to by "dev".
> > + */
> > +#define DEVICE_UPDATE_JSON(type, ptr, cnt, domid, dev, compare, rc)     \
> 
> You could define this in terms of REMOVE+ADD, which would match the
> behaviour of the actual operation. Slightly less efficient but less code
> perhaps?
> 

Correct, it's less efficient. But if you prefer it that way I can make
it that way. I guess one extra read and one extra write won't slow down
things much.

Wei.

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