[PATCH] qemu: implement QEMU NBD source reconnect delay attribute
Christian Nautze
christian.nautze at exoscale.ch
Mon Jan 30 13:36:26 UTC 2023
Thank you, I see your point. We could just ignore the 'enabled' attribute
from the reconnect element
when it's used in the NBD source context:
<source protocol='nbd' name='foo'>
<host name='example.org <http://example.org>' port='6000'/>
<reconnect delay='10'/>
</source>
Another solution would be do add a totally new XML element.
What would be your preferred solution?
Thanks again
Christian
On Fri, 27 Jan 2023 at 23:42, Jonathon Jongsma <jjongsma at redhat.com> wrote:
> On 1/27/23 8:40 AM, Christian Nautze wrote:
> > Hi!
> >
> > Is it possible to get a review on this patch I send before Christmas? :)
> > Thank you!
> >
> > Christian
> >
> > _____________________________________________________
> >
> > Currently it's only possible to set this parameter during domain
> > creation via QEMU commandline passthrough feature.
> > With the new delay attribute it's also possible to set this
> > parameter if you want to attach a new NBD disk
> > using "virsh attach-device domain device.xml" e.g.:
> >
> > <disk type='network' device='disk'>
> > <driver name='qemu' type='raw'/>
> > <source protocol='nbd' name='foo'>
> > <host name='example.org <http://example.org>' port='6000'/>
> > <reconnect enabled='yes' delay='10'/>
>
> In my opinion, this doesn't seem like the ideal syntax for specifying
> this parameter. For example, it allows you to specify something like
>
> <reconnect enabled='no' ... />
>
> which looks like it would disable any attempt by qemu to reconnect to
> the nbd export. But in your code, this just results in not passing any
> value for 'reconnect-delay' to qemu. And I'm pretty sure qemu will still
> attempt to reconnect to the nbd server in this case, but you will lose
> all of the requests made between the disconnection and a successful
> reconnection.
>
> Jonathon
>
>
> > </source>
> > <target dev='vdb' bus='virtio'/>
> > </disk>
> >
> > Signed-off-by: Christian Nautze <christian.nautze at exoscale.ch <
> https://listman.redhat.com/mailman/listinfo/libvir-list>>
> > ---
> > docs/formatdomain.rst | 10 ++++++--
> > src/conf/domain_conf.c | 19 +++++++++++++++
> > src/conf/schemas/domaincommon.rng | 3 +++
> > src/conf/schemas/storagecommon.rng | 5 ++++
> > src/conf/storage_source_conf.c | 1 +
> > src/conf/storage_source_conf.h | 4 ++++
> > src/qemu/qemu_block.c | 4 +++-
> > src/qemu/qemu_domain.c | 9 ++++++++
> > .../disk-network-nbd.x86_64-latest.args | 23 +++++++++++--------
> > tests/qemuxml2argvdata/disk-network-nbd.xml | 8 +++++++
> > tests/qemuxml2xmloutdata/disk-network-nbd.xml | 9 ++++++++
> > 11 files changed, 82 insertions(+), 13 deletions(-)
> >
> > diff --git a/docs/formatdomain.rst b/docs/formatdomain.rst
> > index d7fffc6e0b..3fbeba644a 100644
> > --- a/docs/formatdomain.rst
> > +++ b/docs/formatdomain.rst
> > @@ -2938,13 +2938,19 @@ paravirtualized driver is specified via the
> ``disk`` element.
> > are intended to be default, then the entire element may be
> omitted.
> > ``reconnect``
> > For disk type ``vhostuser`` configures reconnect timeout if the
> connection
> > - is lost. It has two mandatory attributes:
> > + is lost. This is set with the two mandatory attributes
> ``enabled`` and
> > + ``timeout``. For disk type ``network`` and protocol ``nbd`` the
> QEMU NBD
> > + reconnect delay can be set via the mandatory attributes
> ``enabled``
> > + and ``delay``:
> >
> > ``enabled``
> > If the reconnect feature is enabled, accepts ``yes`` and
> ``no``
> > ``timeout``
> > The amount of seconds after which hypervisor tries to
> reconnect.
> > -
> > + ``delay``
> > + The amount of seconds during which all requests are paused and
> will be rerun
> > + after a successful reconnect. After that time, any delayed
> requests and all
> > + future requests before a successful reconnect will immediately
> fail.
> >
> > For a "file" or "volume" disk type which represents a cdrom or
> floppy (the
> > ``device`` attribute), it is possible to define policy what to do
> with the
> > diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
> > index 6c088ff295..909c78ef28 100644
> > --- a/src/conf/domain_conf.c
> > +++ b/src/conf/domain_conf.c
> > @@ -7024,6 +7024,22 @@ virDomainDiskSourceNetworkParse(xmlNodePtr node,
> > src->tlsFromConfig = !!value;
> > }
> >
> > + if (src->protocol == VIR_STORAGE_NET_PROTOCOL_NBD) {
> > + xmlNodePtr cur;
> > + if ((cur = virXPathNode("./reconnect", ctxt))) {
> > + virTristateBool enabled;
> > + if (virXMLPropTristateBool(cur, "enabled",
> VIR_XML_PROP_NONE,
> > + &enabled) < 0)
> > + return -1;
> > +
> > + if (enabled == VIR_TRISTATE_BOOL_YES) {
> > + if (virXMLPropUInt(cur, "delay", 10,
> VIR_XML_PROP_REQUIRED,
> > + &src->reconnectDelay) < 0)
> > + return -1;
> > + }
> > + }
> > + }
> > +
> > /* for historical reasons we store the volume and image name in
> one XML
> > * element although it complicates thing when attempting to access
> them. */
> > if (src->path &&
> > @@ -21729,6 +21745,9 @@ virDomainDiskSourceFormatNetwork(virBuffer
> *attrBuf,
> > virBufferAddLit(childBuf, "/>\n");
> > }
> >
> > + if (src->reconnectDelay) {
> > + virBufferAsprintf(childBuf, "<reconnect enabled='yes'
> delay='%u'/>\n", src->reconnectDelay);
> > + }
> >
> > virBufferEscapeString(childBuf, "<snapshot name='%s'/>\n",
> src->snapshot);
> > virBufferEscapeString(childBuf, "<config file='%s'/>\n",
> src->configFile);
> > diff --git a/src/conf/schemas/domaincommon.rng
> b/src/conf/schemas/domaincommon.rng
> > index c588a48fd2..72587416fe 100644
> > --- a/src/conf/schemas/domaincommon.rng
> > +++ b/src/conf/schemas/domaincommon.rng
> > @@ -2184,6 +2184,9 @@
> > </optional>
> > <ref name="diskSourceCommon"/>
> > <ref name="diskSourceNetworkHost"/>
> > + <optional>
> > + <ref name="reconnect"/>
> > + </optional>
> > <optional>
> > <ref name="encryption"/>
> > </optional>
> > diff --git a/src/conf/schemas/storagecommon.rng
> b/src/conf/schemas/storagecommon.rng
> > index 76714c9aad..2842376e78 100644
> > --- a/src/conf/schemas/storagecommon.rng
> > +++ b/src/conf/schemas/storagecommon.rng
> > @@ -61,6 +61,11 @@
> > <ref name="unsignedInt"/>
> > </attribute>
> > </optional>
> > + <optional>
> > + <attribute name="delay">
> > + <ref name="unsignedInt"/>
> > + </attribute>
> > + </optional>
> > </element>
> > </define>
> >
> > diff --git a/src/conf/storage_source_conf.c
> b/src/conf/storage_source_conf.c
> > index 2b4cf5e241..edfd17c77c 100644
> > --- a/src/conf/storage_source_conf.c
> > +++ b/src/conf/storage_source_conf.c
> > @@ -810,6 +810,7 @@ virStorageSourceCopy(const virStorageSource *src,
> > def->sslverify = src->sslverify;
> > def->readahead = src->readahead;
> > def->timeout = src->timeout;
> > + def->reconnectDelay = src->reconnectDelay;
> > def->metadataCacheMaxSize = src->metadataCacheMaxSize;
> >
> > /* storage driver metadata are not copied */
> > diff --git a/src/conf/storage_source_conf.h
> b/src/conf/storage_source_conf.h
> > index f2440cec6a..b60a4b3346 100644
> > --- a/src/conf/storage_source_conf.h
> > +++ b/src/conf/storage_source_conf.h
> > @@ -290,6 +290,10 @@ struct _virStorageSource {
> > unsigned long long readahead; /* size of the readahead buffer in
> bytes */
> > unsigned long long timeout; /* connection timeout in seconds */
> >
> > + /* NBD QEMU reconnect-delay option,
> > + * 0 as default value */
> > + unsigned int reconnectDelay;
> > +
> > virStorageSourceNVMeDef *nvme; /* type == VIR_STORAGE_TYPE_NVME */
> >
> > virDomainChrSourceDef *vhostuser; /* type ==
> VIR_STORAGE_TYPE_VHOST_USER */
> > diff --git a/src/qemu/qemu_block.c b/src/qemu/qemu_block.c
> > index 8a6f601b29..6913c380a0 100644
> > --- a/src/qemu/qemu_block.c
> > +++ b/src/qemu/qemu_block.c
> > @@ -530,6 +530,7 @@ qemuBlockStorageSourceGetNBDProps(virStorageSource
> *src,
> > "S:export", src->path,
> > "S:tls-creds", tlsAlias,
> > "S:tls-hostname", tlsHostname,
> > + "p:reconnect-delay", src->reconnectDelay,
> > NULL) < 0)
> > return NULL;
> >
> > @@ -1817,7 +1818,8 @@ qemuBlockGetBackingStoreString(virStorageSource
> *src,
> > src->ncookies == 0 &&
> > src->sslverify == VIR_TRISTATE_BOOL_ABSENT &&
> > src->timeout == 0 &&
> > - src->readahead == 0) {
> > + src->readahead == 0 &&
> > + src->reconnectDelay == 0) {
> >
> > switch ((virStorageNetProtocol) src->protocol) {
> > case VIR_STORAGE_NET_PROTOCOL_NBD:
> > diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c
> > index 5c05032ce3..840d857f78 100644
> > --- a/src/qemu/qemu_domain.c
> > +++ b/src/qemu/qemu_domain.c
> > @@ -4927,6 +4927,15 @@ qemuDomainValidateStorageSource(virStorageSource
> *src,
> > }
> > }
> >
> > + if (src->reconnectDelay > 0) {
> > + if (actualType != VIR_STORAGE_TYPE_NETWORK ||
> > + src->protocol != VIR_STORAGE_NET_PROTOCOL_NBD) {
> > + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
> > + _("reconnect delay is supported only with
> NBD protocol"));
> > + return -1;
> > + }
> > + }
> > +
> > if (src->query &&
> > (actualType != VIR_STORAGE_TYPE_NETWORK ||
> > (src->protocol != VIR_STORAGE_NET_PROTOCOL_HTTPS &&
> > diff --git a/tests/qemuxml2argvdata/disk-network-nbd.x86_64-latest.args
> b/tests/qemuxml2argvdata/disk-network-nbd.x86_64-latest.args
> > index 21e619af3e..e8d13b0bd4 100644
> > --- a/tests/qemuxml2argvdata/disk-network-nbd.x86_64-latest.args
> > +++ b/tests/qemuxml2argvdata/disk-network-nbd.x86_64-latest.args
> > @@ -28,21 +28,24 @@
> XDG_CONFIG_HOME=/tmp/lib/domain--1-QEMUGuest1/.config \
> > -no-acpi \
> > -boot strict=on \
> > -device
> '{"driver":"piix3-usb-uhci","id":"usb","bus":"pci.0","addr":"0x1.0x2"}' \
> > --blockdev '{"driver":"nbd","server":{"type":"inet","host":"example.org
> <http://example.org>","port":"6000"},"node-name":"libvirt-5-storage","auto-read-only":true,"discard":"unmap"}'
> \
> > +-blockdev '{"driver":"nbd","server":{"type":"inet","host":"example.org
> <http://example.org>","port":"6000"},"node-name":"libvirt-6-storage","auto-read-only":true,"discard":"unmap"}'
> \
> > +-blockdev
> '{"node-name":"libvirt-6-format","read-only":false,"driver":"raw","file":"libvirt-6-storage"}'
> \
> > +-device
> '{"driver":"virtio-blk-pci","bus":"pci.0","addr":"0x2","drive":"libvirt-6-format","id":"virtio-disk0","bootindex":1}'
> \
> > +-blockdev '{"driver":"nbd","server":{"type":"inet","host":"example.org
> <http://example.org>","port":"6000"},"export":"bar","node-name":"libvirt-5-storage","auto-read-only":true,"discard":"unmap"}'
> \
> > -blockdev
> '{"node-name":"libvirt-5-format","read-only":false,"driver":"raw","file":"libvirt-5-storage"}'
> \
> > --device
> '{"driver":"virtio-blk-pci","bus":"pci.0","addr":"0x2","drive":"libvirt-5-format","id":"virtio-disk0","bootindex":1}'
> \
> > --blockdev '{"driver":"nbd","server":{"type":"inet","host":"example.org
> <http://example.org>","port":"6000"},"export":"bar","node-name":"libvirt-4-storage","auto-read-only":true,"discard":"unmap"}'
> \
> > +-device
> '{"driver":"virtio-blk-pci","bus":"pci.0","addr":"0x3","drive":"libvirt-5-format","id":"virtio-disk1"}'
> \
> > +-blockdev
> '{"driver":"nbd","server":{"type":"inet","host":"::1","port":"6000"},"node-name":"libvirt-4-storage","auto-read-only":true,"discard":"unmap"}'
> \
> > -blockdev
> '{"node-name":"libvirt-4-format","read-only":false,"driver":"raw","file":"libvirt-4-storage"}'
> \
> > --device
> '{"driver":"virtio-blk-pci","bus":"pci.0","addr":"0x3","drive":"libvirt-4-format","id":"virtio-disk1"}'
> \
> > --blockdev
> '{"driver":"nbd","server":{"type":"inet","host":"::1","port":"6000"},"node-name":"libvirt-3-storage","auto-read-only":true,"discard":"unmap"}'
> \
> > +-device
> '{"driver":"virtio-blk-pci","bus":"pci.0","addr":"0x4","drive":"libvirt-4-format","id":"virtio-disk2"}'
> \
> > +-blockdev
> '{"driver":"nbd","server":{"type":"inet","host":"::1","port":"6000"},"export":"bar","node-name":"libvirt-3-storage","auto-read-only":true,"discard":"unmap"}'
> \
> > -blockdev
> '{"node-name":"libvirt-3-format","read-only":false,"driver":"raw","file":"libvirt-3-storage"}'
> \
> > --device
> '{"driver":"virtio-blk-pci","bus":"pci.0","addr":"0x4","drive":"libvirt-3-format","id":"virtio-disk2"}'
> \
> > --blockdev
> '{"driver":"nbd","server":{"type":"inet","host":"::1","port":"6000"},"export":"bar","node-name":"libvirt-2-storage","auto-read-only":true,"discard":"unmap"}'
> \
> > +-device
> '{"driver":"virtio-blk-pci","bus":"pci.0","addr":"0x5","drive":"libvirt-3-format","id":"virtio-disk3"}'
> \
> > +-blockdev
> '{"driver":"nbd","server":{"type":"unix","path":"/var/run/nbdsock"},"export":"bar","node-name":"libvirt-2-storage","auto-read-only":true,"discard":"unmap"}'
> \
> > -blockdev
> '{"node-name":"libvirt-2-format","read-only":false,"driver":"raw","file":"libvirt-2-storage"}'
> \
> > --device
> '{"driver":"virtio-blk-pci","bus":"pci.0","addr":"0x5","drive":"libvirt-2-format","id":"virtio-disk3"}'
> \
> > --blockdev
> '{"driver":"nbd","server":{"type":"unix","path":"/var/run/nbdsock"},"export":"bar","node-name":"libvirt-1-storage","auto-read-only":true,"discard":"unmap"}'
> \
> > +-device
> '{"driver":"virtio-blk-pci","bus":"pci.0","addr":"0x6","drive":"libvirt-2-format","id":"virtio-disk4"}'
> \
> > +-blockdev '{"driver":"nbd","server":{"type":"inet","host":"example.org
> <http://example.org>","port":"6000"},"export":"foo","reconnect-delay":10,"node-name":"libvirt-1-storage","auto-read-only":true,"discard":"unmap"}'
> \
> > -blockdev
> '{"node-name":"libvirt-1-format","read-only":false,"driver":"raw","file":"libvirt-1-storage"}'
> \
> > --device
> '{"driver":"virtio-blk-pci","bus":"pci.0","addr":"0x6","drive":"libvirt-1-format","id":"virtio-disk4"}'
> \
> > +-device
> '{"driver":"virtio-blk-pci","bus":"pci.0","addr":"0x7","drive":"libvirt-1-format","id":"virtio-disk5"}'
> \
> > -audiodev '{"id":"audio1","driver":"none"}' \
> > -sandbox
> on,obsolete=deny,elevateprivileges=deny,spawn=deny,resourcecontrol=deny \
> > -msg timestamp=on
> > diff --git a/tests/qemuxml2argvdata/disk-network-nbd.xml
> b/tests/qemuxml2argvdata/disk-network-nbd.xml
> > index 8ac6cc3b7b..243ffeb1ed 100644
> > --- a/tests/qemuxml2argvdata/disk-network-nbd.xml
> > +++ b/tests/qemuxml2argvdata/disk-network-nbd.xml
> > @@ -49,6 +49,14 @@
> > </source>
> > <target dev='vde' bus='virtio'/>
> > </disk>
> > + <disk type='network' device='disk'>
> > + <driver name='qemu' type='raw'/>
> > + <source protocol='nbd' name='foo'>
> > + <host name='example.org <http://example.org>' port='6000'/>
> > + <reconnect enabled='yes' delay='10'/>
> > + </source>
> > + <target dev='vdf' bus='virtio'/>
> > + </disk>
> > <controller type='usb' index='0'/>
> > <controller type='ide' index='0'/>
> > <controller type='pci' index='0' model='pci-root'/>
> > diff --git a/tests/qemuxml2xmloutdata/disk-network-nbd.xml
> b/tests/qemuxml2xmloutdata/disk-network-nbd.xml
> > index f8dcca4bab..ed0c760d5b 100644
> > --- a/tests/qemuxml2xmloutdata/disk-network-nbd.xml
> > +++ b/tests/qemuxml2xmloutdata/disk-network-nbd.xml
> > @@ -54,6 +54,15 @@
> > <target dev='vde' bus='virtio'/>
> > <address type='pci' domain='0x0000' bus='0x00' slot='0x06'
> function='0x0'/>
> > </disk>
> > + <disk type='network' device='disk'>
> > + <driver name='qemu' type='raw'/>
> > + <source protocol='nbd' name='foo'>
> > + <host name='example.org <http://example.org>' port='6000'/>
> > + <reconnect enabled='yes' delay='10'/>
> > + </source>
> > + <target dev='vdf' bus='virtio'/>
> > + <address type='pci' domain='0x0000' bus='0x00' slot='0x07'
> function='0x0'/>
> > + </disk>
> > <controller type='usb' index='0'>
> > <address type='pci' domain='0x0000' bus='0x00' slot='0x01'
> function='0x2'/>
> > </controller>
> > --
> > 2.34.1
> >
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://listman.redhat.com/archives/libvir-list/attachments/20230130/9b7214d9/attachment-0001.htm>
More information about the libvir-list
mailing list