Re: [Xen-devel] [PATCH v2 13/18] libxl: make libxl_cd_insert "eject" + "insert"

[Ian Campbell <ian.campbell@xxxxxxxxxx>]

On Thu, 2014-08-28 at 12:25 +0100, Wei Liu wrote:
> On Wed, Aug 27, 2014 at 03:04:46AM +0100, Ian Campbell wrote:
> > On Wed, 2014-07-30 at 19:23 +0100, Wei Liu wrote:
> > > A "cdrom insert" is always processed as "eject" + "insert", with JSON
> > > config updated in between. So that we can know the correct state of
> > > CDROM later when we try to retrieve domain configuration: if xenstore is
> > > "empty", then CDROM is "empty"; otherwise use the information presented
> > > in JSON.
> > 
> > When you say "empty" do you mean literally that the xenstore nodes do
> > not exist or that one of them contains the string "empty"? If, as I
> > suspect, you mean the former then writing 
> >         if xenstore nodes are not present then CDROM is "empty"
> > would confuse me less.
> > 
> 
> The xenstore node still exists, just that "params" doesn't point to
> media.

"doesn't point to" means doesn't exist or contains the empty string? I
think it would be good to be explicit there.

>  I can rephrase this:
> 
>   If xenstore entry doesn't point to media file then CDROM is "empty".
> 
> > > +    rc = libxl__cdrom_insert(ctx, domid, &empty, NULL);
> > > +    if (rc)
> > > +        goto out;
> > > +
> > > +    /* Optimisation: don't insert empty disk twice, and skip
> > > +     * manipulating JSON. */
> > 
> > Do we not need to update the JSON in this case to reflect that it is now
> > empty?
> > 
> 
> We could but we don't have to -- xenstore is primary reference.

OK.



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