RE: [PATCH 1/2] conf: add locking option to disk source

On Wed, Feb 14, 2024 at 13:17:39 +0100, Peter Krempa wrote: > On Wed, Feb 14, 2024 at 14:17:57 +0900, Hiroki Narukawa wrote: > > There is a case that locking hits a bug and users wants to disable > > locking like bug in Linux kernel. > > > > This commit adds option to configure locking for file source. > > > > Signed-off-by: Hiroki Narukawa <hnarukaw(a)yahoo-corp.jp&gt; > > --- > > docs/formatdomain.rst | 5 +++++ > > src/conf/domain_conf.c | 8 ++++++++ > > src/conf/schemas/domaincommon.rng | 5 +++++ > > src/conf/storage_source_conf.h | 3 +++ > > 4 files changed, 21 insertions(+) > > > > diff --git a/docs/formatdomain.rst b/docs/formatdomain.rst index > > 0a4f9d9000..95869d573c 100644 > > --- a/docs/formatdomain.rst > > +++ b/docs/formatdomain.rst > > @@ -2782,6 +2782,11 @@ paravirtualized driver is specified via the ``disk`` element. > > via the filesystem. The filename passed via ``file`` can still be used > > to generate paths to write into image metadata when doing block operations > > but libvirt will not access these natively. > > + > > + :since:`Since 10.1.0` a new optional attribute ``locking`` can be added > > + which can take "on" or "off". Basically this option should be kept > > + default, but it can be used explicitly for workaround about bug in > > + locking. > > This is not the first time this is being suggested and I'm still not > persuaded that we want this. We definitelly do not want this as a > generic production attribute. > > A possibility would be to do it as part of the qemu XML namespace > along with tainting the VM that such config is being used. > > https://libv/ > irt.org%2Fdrvqemu.html%23pass-through-of-arbitrary-qemu-commands&data= > 05%7C02%7Chnarukaw%40yahoo-corp.jp%7Ce8a8b57666244c52e3b108dc2d58 bcf2% > 7Ca208d369cd4e4f87b11998eaf31df2c3%7C1%7C0%7C638435106350299392%7 CUnkn > own%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1h aWwi > LCJXVCI6Mn0%3D%7C0%7C%7C%7C&sdata=MIFzrihGDgVq4XO8na2K%2F7c9q 708wMwKAZ > 0yKU1ZJVE%3D&reserved=0 > > The main reason is that you never actually want to use this, except > for the very specific case of bugs you are mentioning. To elaborate on the qemu XML namespace integration: Currently we parse the namespace elements only on a global scope, so per-disk XML attributes are not supported. Thus to add per-disk or per-device XML namespace attributes a new parsing callback will be needed. A second option would be to do it similarly to how we allow override of device parameters: https://libvirt.org/ %2Fdrvqemu.html%23overriding-properties-of-qemu-devices&data=05%7C02% 7Chnarukaw%40yahoo-corp.jp%7Ce8a8b57666244c52e3b108dc2d58bcf2%7Ca2 08d369cd4e4f87b11998eaf31df2c3%7C1%7C0%7C638435106350305499%7CUnk nown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1 haWwiLCJXVCI6Mn0%3D%7C0%7C%7C%7C&sdata=7eH0zujLapnHQRIQXqihwP ADhiAt77yInaSVjUdFez4%3D&reserved=0 The image to override would be identified using the TARGET[INDEX] tuple (e.g. vda[1]) as declared in the XML.