Re: [Xen-devel] [PATCH 6/9] libxl_disk: Cut libxl_cdrom_insert into steps ..

[Anthony PERARD <anthony.perard@citrix.com>]

On Tue, Jun 04, 2019 at 06:29:14PM +0100, Ian Jackson wrote:
> Anthony PERARD writes ("[PATCH 6/9] libxl_disk: Cut libxl_cdrom_insert into steps .."):
> > This patch cut libxl_cdrom_insert into different step/function but there
> > are still called synchronously. A later patch will call them
> > asynchronously when QMP is involved.
> > 
> > The json_lock has been replaced by the qmp_lock for protection against
> > concurrent changes to the cdrom. The json_lock is now only used when
> > reading/modifying the domain userdata.
> 
> 
> Your documentation for the qmp lock, taken roughly from my idea, is
> that the qmp lock covers modifying things via qmp.  But I think here
> you use it for updates via xenstore ?

That xenstore code update is kind of weird, libxl writes it but nothing
really reads it.

For the eject part of the xenstore update, it probably doesn't matter if
the json_lock is held or not. But for the insert part of the xenstore
update, libxl writes the domain config while in the middle of the xenstore
transaction. So in the second case, the json_lock needs to be held.

> I think this is OK because what matters is that any one thing is
> always covered by the same lock - and here the cdrom is is a "thing".
> But I think this means the commentary is wrong ?

I can try to rewrite the commentary to make it less wrong. Or change the
code so the comment applies to it. (I'll go with rewriting the patch
description for now.)

-- 
Anthony PERARD

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