Hi, I know everyone is drowning in work but perhaps someone could review this patch? I think this is also a useful feature for others to configure the reconnect delay while attaching a NBD disk to a running VM. Thanks! Christian
On Mon, 27 Feb 2023 at 12:15, Christian Nautze <christian.nau...@exoscale.ch> wrote: > 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' port='6000'/> > <reconnect delay='10'/> > </source> > <target dev='vdb' bus='virtio'/> > </disk> > > Signed-off-by: Christian Nautze <christian.nau...@exoscale.ch> > > --- > Changes: schema reconnect element: drop mandatory 'enabled' attribute when > using 'delay' > reconnect element: use choice for attributes > --- > docs/formatdomain.rst | 11 +++++++-- > src/conf/domain_conf.c | 12 ++++++++++ > src/conf/schemas/domaincommon.rng | 3 +++ > src/conf/schemas/storagecommon.rng | 19 ++++++++++----- > 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, 84 insertions(+), 19 deletions(-) > > diff --git a/docs/formatdomain.rst b/docs/formatdomain.rst > index 638768c18d..e4ea330a0d 100644 > --- a/docs/formatdomain.rst > +++ b/docs/formatdomain.rst > @@ -2946,13 +2946,20 @@ 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 attribute ``delay``: > > ``enabled`` > If the reconnect feature is enabled, accepts ``yes`` and ``no`` > ``timeout`` > The amount of seconds after which hypervisor tries to reconnect. > - > + ``delay`` > + Only for NBD hosts. 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. If not set the default QEMU value is 0. > > 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 84ffd93b7f..1e5c88e45d 100644 > --- a/src/conf/domain_conf.c > +++ b/src/conf/domain_conf.c > @@ -7146,6 +7146,15 @@ virDomainDiskSourceNetworkParse(xmlNodePtr node, > src->tlsFromConfig = !!value; > } > > + if (src->protocol == VIR_STORAGE_NET_PROTOCOL_NBD) { > + xmlNodePtr cur; > + if ((cur = virXPathNode("./reconnect", ctxt))) { > + if (virXMLPropUInt(cur, "delay", 10, VIR_XML_PROP_NONE, > + &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 && > @@ -22087,6 +22096,9 @@ virDomainDiskSourceFormatNetwork(virBuffer > *attrBuf, > virBufferAddLit(childBuf, "/>\n"); > } > > + if (src->reconnectDelay) { > + virBufferAsprintf(childBuf, "<reconnect 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 ab4886b783..03aab6f7e5 100644 > --- a/src/conf/schemas/domaincommon.rng > +++ b/src/conf/schemas/domaincommon.rng > @@ -2199,6 +2199,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 4d6e646c9a..23eff9ecb1 100644 > --- a/src/conf/schemas/storagecommon.rng > +++ b/src/conf/schemas/storagecommon.rng > @@ -55,14 +55,21 @@ > > <define name="reconnect"> > <element name="reconnect"> > - <attribute name="enabled"> > - <ref name="virYesNo"/> > - </attribute> > - <optional> > - <attribute name="timeout"> > + <choice> > + <group> > + <attribute name="enabled"> > + <ref name="virYesNo"/> > + </attribute> > + <optional> > + <attribute name="timeout"> > + <ref name="unsignedInt"/> > + </attribute> > + </optional> > + </group> > + <attribute name="delay"> > <ref name="unsignedInt"/> > </attribute> > - </optional> > + </choice> > </element> > </define> > > diff --git a/src/conf/storage_source_conf.c > b/src/conf/storage_source_conf.c > index cecd7e811e..58009fd06e 100644 > --- a/src/conf/storage_source_conf.c > +++ b/src/conf/storage_source_conf.c > @@ -811,6 +811,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 14a6825d54..c6187dda59 100644 > --- a/src/conf/storage_source_conf.h > +++ b/src/conf/storage_source_conf.h > @@ -312,6 +312,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 5e700eff99..8fcebd8992 100644 > --- a/src/qemu/qemu_block.c > +++ b/src/qemu/qemu_block.c > @@ -529,6 +529,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; > > @@ -1848,7 +1849,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 4cf9a259ea..1206d59f4a 100644 > --- a/src/qemu/qemu_domain.c > +++ b/src/qemu/qemu_domain.c > @@ -5020,6 +5020,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","port":"6000"},"node-name":"libvirt-5-storage","auto-read-only":true,"discard":"unmap"}' > \ > +-blockdev > '{"driver":"nbd","server":{"type":"inet","host":"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","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","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","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..4e8b1e5b03 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' port='6000'/> > + <reconnect 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..38d1f290c8 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' port='6000'/> > + <reconnect 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 > >