[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Xen-devel] [PATCH] libxl: Documentation about the domain configuration on disk



On Thu, Dec 06, 2018 at 12:16:40PM +0000, Wei Liu wrote:
> On Thu, Dec 06, 2018 at 10:43:32AM +0000, Anthony PERARD wrote:
> > +UPDATE OF DOMAIN CONFIGURATION
> > +------------------------------
> > +
> > +Also known as "libxl-json" userdata or `libxl_domain_config'.
> > +
> > +Whenever a running domain have its configuration updated, like changing
> > +media in a cdrom drive, the domain configuration in libxl private data
> > +store needs to be updated as well. The domain configuration should
> > +contain *more* information about the domain rather than less, stale data
> > +are easier to spot that missing data.
> > +
> > +Here is an example of how to update the domain configuration:
> > + * Remove current media from cdrom drive
> > + * Update domain configuration with media removed
> 
> We may not even need this because the primary source in this case is
> QEMU. See below.
> 
> > + ( we could stop here)
> > + * Update domain configuration to add media we are about to insert
> > + * Insert media into cdrom drive
> 
> In essence we need a primary reference while using libxl-json file as a
> secondary source.
> 
> When doing device hotplug, the primary source is xenstore. It may become
> QEMU in the future if we move to a model where everything is
> communicated via QMP.
> 
> When doing CDROM insertion and rejection, the primary source is QEMU
> state.

I'm not trying to figure out what primary source should be here, I'm
trying to find out how the secondary source, namely "libxl-json", should
behave, what it should contain, when to update it compare the primary
source (what a guest ultimately see).

> All in all I think your description is not wrong but it failed to
> capture the high-level intent -- always update libxl-json before
> updating the primary source.

That isn't what Ian said IRL, I don't think. From what I understand,
when removing a media/disk, first remove the media, then update
libxl-json; when adding a media/disk, first update libxl-json, then add
the media.

> > +
> > +Retrieve / store domain configuration from / to libxl private data store
> > +are done with `libxl__get_domain_configuration' and
> > +`libxl__set_domain_configuration'. Consult libxl_internal.h for more
> > +information.
> > +
> 
> What do you think about the text around libxl_internal.h:L2598?

If only I knew this comment existed :-(. It is burried, don't mention
"libxl-json" or "userdata" or "domain config" but only the not very
helpful term "json config"... Hmm, ... it actualy have "domain
configuration" once.

Anyway, that comment block isn't very helpful because it basically says
that we can't depriv QEMU, I mean do hotplug with a deprived QEMU. It
assumes that we can keep a lock on the userdata while updating the
guest, but we can't keep the lock while talking with QEMU (or more
generaly: we can't keep the lock while doing any async operation).

But there is one useful piece of information:
    Here we maintain one invariant: every device in xenstore must have
    an entry in JSON file.
(xenstore is describe as "primary reference" just before that sentence).

This is what I would like my past self to be able to find out more
easly, and having the information in CODING_STYLE would make sense I
think.

> Maybe we should extend that comment block?

I still think it would be helpful to have pointers in CODING_STYLE, as
there isn't a single place in libxl_internal.h where the information I
was looking for could be added.

Thanks,

-- 
Anthony PERARD

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxxx
https://lists.xenproject.org/mailman/listinfo/xen-devel

 


Rackspace

Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.