From: Peter Krempa <pkrempa@redhat.com> Use a 'switch' statement instead of a bunch of if statements to do validation and selection what to parse. Remove the pre-clearing of the struct as we always alocate cleared memory for it and we can reorder assignments to avoid the need for cleanup. Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/conf/domain_conf.c | 68 ++++++++++++++++++++++-------------------- 1 file changed, 36 insertions(+), 32 deletions(-) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 53ab502306..1680da3ea6 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -6016,55 +6016,59 @@ int virDomainStorageNetworkParseHost(xmlNodePtr hostnode, virStorageNetHostDef *host) { - int ret = -1; - g_autofree char *port = NULL; - - memset(host, 0, sizeof(*host)); + g_autofree char *socket = NULL; if (virXMLPropEnumDefault(hostnode, "transport", virStorageNetHostTransportTypeFromString, VIR_XML_PROP_NONE, &host->transport, - VIR_STORAGE_NET_HOST_TRANS_TCP) < 0) { - goto cleanup; - } + VIR_STORAGE_NET_HOST_TRANS_TCP) < 0) + return -1; - host->socket = virXMLPropString(hostnode, "socket"); + socket = virXMLPropString(hostnode, "socket"); - if (host->transport == VIR_STORAGE_NET_HOST_TRANS_UNIX && - host->socket == NULL) { - virReportError(VIR_ERR_XML_ERROR, "%s", - _("missing socket for unix transport")); - goto cleanup; - } + switch (host->transport) { + case VIR_STORAGE_NET_HOST_TRANS_UNIX: + if (!socket) { + virReportError(VIR_ERR_XML_ERROR, "%s", + _("missing socket for unix transport")); + return -1; + } - if (host->transport != VIR_STORAGE_NET_HOST_TRANS_UNIX && - host->socket != NULL) { - virReportError(VIR_ERR_XML_ERROR, - _("transport '%1$s' does not support socket attribute"), - virStorageNetHostTransportTypeToString(host->transport)); - goto cleanup; - } + host->socket = g_steal_pointer(&socket); + break; + + case VIR_STORAGE_NET_HOST_TRANS_TCP: + case VIR_STORAGE_NET_HOST_TRANS_RDMA: { + g_autofree char *portstr = NULL; + unsigned int port = 0; + + if (socket) { + virReportError(VIR_ERR_XML_ERROR, + _("transport '%1$s' does not support socket attribute"), + virStorageNetHostTransportTypeToString(host->transport)); + return -1; + } + + if ((portstr = virXMLPropString(hostnode, "port")) && + virStringParsePort(portstr, &port) < 0) + return -1; - if (host->transport != VIR_STORAGE_NET_HOST_TRANS_UNIX) { if (!(host->name = virXMLPropString(hostnode, "name"))) { virReportError(VIR_ERR_XML_ERROR, "%s", _("missing name for host")); - goto cleanup; + return -1; } - if ((port = virXMLPropString(hostnode, "port"))) { - if (virStringParsePort(port, &host->port) < 0) - goto cleanup; - } + host->port = port; } + break; - ret = 0; + case VIR_STORAGE_NET_HOST_TRANS_LAST: + break; + } - cleanup: - if (ret < 0) - virStorageNetHostDefClear(host); - return ret; + return 0; } -- 2.49.0

From: Peter Krempa <pkrempa@redhat.com> Prepare the parser code and anything using 'virStorageNetHostTransport' to support passing a FD instead of opening the connection by qemu itself. For now this just preparse the parser and data structures, but the code is dormant. Only code paths which will actually support FD passing will then enable it in the future. Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/conf/backup_conf.c | 2 +- src/conf/domain_conf.c | 24 +++++++++++++++++-- src/conf/domain_conf.h | 3 ++- src/conf/storage_source_conf.c | 2 ++ src/conf/storage_source_conf.h | 3 +++ src/qemu/qemu_backup.c | 1 + src/qemu/qemu_block.c | 1 + src/qemu/qemu_monitor_json.c | 1 + .../storage_file_backend_gluster.c | 1 + 9 files changed, 34 insertions(+), 4 deletions(-) diff --git a/src/conf/backup_conf.c b/src/conf/backup_conf.c index 1fea6a2be7..1bdfbfa3d6 100644 --- a/src/conf/backup_conf.c +++ b/src/conf/backup_conf.c @@ -228,7 +228,7 @@ virDomainBackupDefParseXML(xmlXPathContextPtr ctxt, def->server = g_new0(virStorageNetHostDef, 1); - if (virDomainStorageNetworkParseHost(node, def->server) < 0) + if (virDomainStorageNetworkParseHost(node, def->server, false) < 0) return NULL; if (def->server->transport == VIR_STORAGE_NET_HOST_TRANS_RDMA) { diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 1680da3ea6..c724638180 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -6014,7 +6014,8 @@ virDomainHostdevSubsysPCIDefParseXML(xmlNodePtr node, int virDomainStorageNetworkParseHost(xmlNodePtr hostnode, - virStorageNetHostDef *host) + virStorageNetHostDef *host, + bool allow_fd) { g_autofree char *socket = NULL; @@ -6064,6 +6065,25 @@ virDomainStorageNetworkParseHost(xmlNodePtr hostnode, } break; + case VIR_STORAGE_NET_HOST_TRANS_FD: + if (!allow_fd) { + virReportError(VIR_ERR_XML_ERROR, "%s", + _("transport 'fd' is now allowed")); + return -1; + } + + if (socket) { + virReportError(VIR_ERR_XML_ERROR, + _("transport '%1$s' does not support socket attribute"), + virStorageNetHostTransportTypeToString(host->transport)); + return -1; + } + + if (!(host->fdgroup = virXMLPropStringRequired(hostnode, "fdgroup"))) + return -1; + + break; + case VIR_STORAGE_NET_HOST_TRANS_LAST: break; } @@ -6092,7 +6112,7 @@ virDomainStorageNetworkParseHosts(xmlNodePtr node, *nhosts = nhostnodes; for (i = 0; i < nhostnodes; i++) { - if (virDomainStorageNetworkParseHost(hostnodes[i], *hosts + i) < 0) + if (virDomainStorageNetworkParseHost(hostnodes[i], *hosts + i, false) < 0) return -1; } diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h index 0d1dd954ae..8dfadbb98d 100644 --- a/src/conf/domain_conf.h +++ b/src/conf/domain_conf.h @@ -4024,7 +4024,8 @@ virDomainDiskByTarget(virDomainDef *def, void virDomainDiskInsert(virDomainDef *def, virDomainDiskDef *disk); int virDomainStorageNetworkParseHost(xmlNodePtr hostnode, - virStorageNetHostDef *host); + virStorageNetHostDef *host, + bool allow_fd); int virDomainDiskDefAssignAddress(virDomainXMLOption *xmlopt, virDomainDiskDef *def, const virDomainDef *vmdef); diff --git a/src/conf/storage_source_conf.c b/src/conf/storage_source_conf.c index ca956a1b7c..db12dfd961 100644 --- a/src/conf/storage_source_conf.c +++ b/src/conf/storage_source_conf.c @@ -97,6 +97,7 @@ VIR_ENUM_IMPL(virStorageNetHostTransport, "tcp", "unix", "rdma", + "fd", ); @@ -149,6 +150,7 @@ virStorageNetHostDefClear(virStorageNetHostDef *def) VIR_FREE(def->name); VIR_FREE(def->socket); + VIR_FREE(def->fdgroup); } diff --git a/src/conf/storage_source_conf.h b/src/conf/storage_source_conf.h index e6cbb93c06..52ed303657 100644 --- a/src/conf/storage_source_conf.h +++ b/src/conf/storage_source_conf.h @@ -141,6 +141,7 @@ typedef enum { VIR_STORAGE_NET_HOST_TRANS_TCP, VIR_STORAGE_NET_HOST_TRANS_UNIX, VIR_STORAGE_NET_HOST_TRANS_RDMA, + VIR_STORAGE_NET_HOST_TRANS_FD, VIR_STORAGE_NET_HOST_TRANS_LAST } virStorageNetHostTransport; @@ -154,6 +155,8 @@ struct _virStorageNetHostDef { unsigned int port; virStorageNetHostTransport transport; char *socket; /* path to unix socket */ + + char *fdgroup; }; diff --git a/src/qemu/qemu_backup.c b/src/qemu/qemu_backup.c index 43576d135b..2935153cdf 100644 --- a/src/qemu/qemu_backup.c +++ b/src/qemu/qemu_backup.c @@ -87,6 +87,7 @@ qemuBackupPrepare(virDomainBackupDef *def) break; case VIR_STORAGE_NET_HOST_TRANS_RDMA: + case VIR_STORAGE_NET_HOST_TRANS_FD: case VIR_STORAGE_NET_HOST_TRANS_LAST: virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("unexpected transport in <domainbackup>")); diff --git a/src/qemu/qemu_block.c b/src/qemu/qemu_block.c index e370411e16..3ed279af28 100644 --- a/src/qemu/qemu_block.c +++ b/src/qemu/qemu_block.c @@ -266,6 +266,7 @@ qemuBlockStorageSourceBuildJSONSocketAddress(virStorageNetHostDef *host) break; case VIR_STORAGE_NET_HOST_TRANS_RDMA: + case VIR_STORAGE_NET_HOST_TRANS_FD: case VIR_STORAGE_NET_HOST_TRANS_LAST: virReportError(VIR_ERR_INTERNAL_ERROR, _("transport protocol '%1$s' is not yet supported"), diff --git a/src/qemu/qemu_monitor_json.c b/src/qemu/qemu_monitor_json.c index d0de48fb18..994cf53d9f 100644 --- a/src/qemu/qemu_monitor_json.c +++ b/src/qemu/qemu_monitor_json.c @@ -6342,6 +6342,7 @@ qemuMonitorJSONNBDServerStart(qemuMonitor *mon, addr = qemuMonitorJSONBuildUnixSocketAddress(server->socket); break; case VIR_STORAGE_NET_HOST_TRANS_RDMA: + case VIR_STORAGE_NET_HOST_TRANS_FD: case VIR_STORAGE_NET_HOST_TRANS_LAST: virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("invalid server address")); diff --git a/src/storage_file/storage_file_backend_gluster.c b/src/storage_file/storage_file_backend_gluster.c index 950f8e81fe..df4df0f128 100644 --- a/src/storage_file/storage_file_backend_gluster.c +++ b/src/storage_file/storage_file_backend_gluster.c @@ -74,6 +74,7 @@ virStorageFileBackendGlusterInitServer(virStorageFileBackendGlusterPriv *priv, hoststr = host->socket; break; + case VIR_STORAGE_NET_HOST_TRANS_FD: case VIR_STORAGE_NET_HOST_TRANS_LAST: break; } -- 2.49.0

From: Peter Krempa <pkrempa@redhat.com> There are no other callers. Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/conf/storage_source_conf.c | 19 +++++-------------- src/conf/storage_source_conf.h | 3 --- src/libvirt_private.syms | 1 - 3 files changed, 5 insertions(+), 18 deletions(-) diff --git a/src/conf/storage_source_conf.c b/src/conf/storage_source_conf.c index db12dfd961..9f28580406 100644 --- a/src/conf/storage_source_conf.c +++ b/src/conf/storage_source_conf.c @@ -142,18 +142,6 @@ virStorageSourceHasBacking(const virStorageSource *src) } -void -virStorageNetHostDefClear(virStorageNetHostDef *def) -{ - if (!def) - return; - - VIR_FREE(def->name); - VIR_FREE(def->socket); - VIR_FREE(def->fdgroup); -} - - void virStorageNetHostDefFree(size_t nhosts, virStorageNetHostDef *hosts) @@ -163,8 +151,11 @@ virStorageNetHostDefFree(size_t nhosts, if (!hosts) return; - for (i = 0; i < nhosts; i++) - virStorageNetHostDefClear(&hosts[i]); + for (i = 0; i < nhosts; i++) { + g_free(hosts[i].name); + g_free(hosts[i].socket); + g_free(hosts[i].fdgroup); + } g_free(hosts); } diff --git a/src/conf/storage_source_conf.h b/src/conf/storage_source_conf.h index 52ed303657..de7895e637 100644 --- a/src/conf/storage_source_conf.h +++ b/src/conf/storage_source_conf.h @@ -505,9 +505,6 @@ virSecurityDeviceLabelDef * virStorageSourceGetSecurityLabelDef(virStorageSource *src, const char *model); -void -virStorageNetHostDefClear(virStorageNetHostDef *def); - void virStorageNetHostDefFree(size_t nhosts, virStorageNetHostDef *hosts); diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index 812fa4e435..a8ebf9efd8 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -1153,7 +1153,6 @@ virStorageFileFeatureTypeFromString; virStorageFileFeatureTypeToString; virStorageFileFormatTypeFromString; virStorageFileFormatTypeToString; -virStorageNetHostDefClear; virStorageNetHostDefCopy; virStorageNetHostDefFree; virStorageNetHostTransportTypeFromString; -- 2.49.0

From: Peter Krempa <pkrempa@redhat.com> In deployments where libvirt is containerized together with the VM it may be hard for the management application to access listening sockets inside the container from the outside. This patch implements "transport='fd'" for the NBD server definition for backups which allows to use the existing "virDomainFDAssociate()" to pass FD to a pre-opened server socket to qemu instead of trying to create it by qemu. Add schema, enable the parser, add formatter and implemet the actual passing for the qemu backup code. Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- docs/formatbackup.rst | 21 +++++++++++++++++++++ src/conf/backup_conf.c | 3 ++- src/conf/schemas/domainbackup.rng | 6 ++++++ src/qemu/qemu_backup.c | 27 +++++++++++++++++++++++++++ 4 files changed, 56 insertions(+), 1 deletion(-) diff --git a/docs/formatbackup.rst b/docs/formatbackup.rst index 02847fd5d4..155a45a22f 100644 --- a/docs/formatbackup.rst +++ b/docs/formatbackup.rst @@ -1,3 +1,5 @@ + .. role:: since + Backup XML format ================= @@ -42,6 +44,25 @@ were supplied). The following child elements and attributes are supported: necessary to set up an NBD server that exposes the content of each disk at the time the backup is started. + In addition to the above the NBD server used for backups allows using + ``transport='fd' fdgroup='NAME'`` where ``NAME`` is the name used with + ``virDomainFDAssociate()`` to pass a pre-opened server socket file descriptor + to qemu. :since:`Since 11.3.0` + + Example code to pass a socket with libvirt-python bindings:: + + import socket + import libvirt + + s = socket.socket(socket.AF_UNIX, socket.SOCK_STREAM) + s.bind("/path/to/socket") + + fdlist = [ s.fileno() ] + + conn = libvirt.open() + dom = conn.lookupByName("VMNAME") + dom.FDAssociate("NAME", fdlist) + Note that for the QEMU hypervisor the TLS environment in controlled using ``backup_tls_x509_cert_dir``, ``backup_tls_x509_verify``, and ``backup_tls_x509_secret_uuid`` properties in ``/etc/libvirt/qemu.conf``. diff --git a/src/conf/backup_conf.c b/src/conf/backup_conf.c index 1bdfbfa3d6..b20292af3d 100644 --- a/src/conf/backup_conf.c +++ b/src/conf/backup_conf.c @@ -228,7 +228,7 @@ virDomainBackupDefParseXML(xmlXPathContextPtr ctxt, def->server = g_new0(virStorageNetHostDef, 1); - if (virDomainStorageNetworkParseHost(node, def->server, false) < 0) + if (virDomainStorageNetworkParseHost(node, def->server, true) < 0) return NULL; if (def->server->transport == VIR_STORAGE_NET_HOST_TRANS_RDMA) { @@ -388,6 +388,7 @@ virDomainBackupDefFormat(virBuffer *buf, if (def->server->port) virBufferAsprintf(&serverAttrBuf, " port='%u'", def->server->port); virBufferEscapeString(&serverAttrBuf, " socket='%s'", def->server->socket); + virBufferEscapeString(&serverAttrBuf, " fdgroup='%s'", def->server->fdgroup); } virXMLFormatElement(&childBuf, "server", &serverAttrBuf, NULL); diff --git a/src/conf/schemas/domainbackup.rng b/src/conf/schemas/domainbackup.rng index 80ba155aad..91cf2a7bbd 100644 --- a/src/conf/schemas/domainbackup.rng +++ b/src/conf/schemas/domainbackup.rng @@ -90,6 +90,12 @@ <ref name="absFilePath"/> </attribute> </group> + <group> + <attribute name="transport"> + <value>fd</value> + </attribute> + <attribute name="fdgroup"/> + </group> </choice> </element> <ref name="backupDisksPull"/> diff --git a/src/qemu/qemu_backup.c b/src/qemu/qemu_backup.c index 2935153cdf..f6ee31dc2a 100644 --- a/src/qemu/qemu_backup.c +++ b/src/qemu/qemu_backup.c @@ -761,6 +761,7 @@ qemuBackupBegin(virDomainObj *vm, bool reuse = (flags & VIR_DOMAIN_BACKUP_BEGIN_REUSE_EXTERNAL); int rc = 0; int ret = -1; + g_autoptr(qemuFDPassDirect) fdpass = NULL; virCheckFlags(VIR_DOMAIN_BACKUP_BEGIN_REUSE_EXTERNAL, -1); @@ -847,6 +848,29 @@ qemuBackupBegin(virDomainObj *vm, priv->backup = g_steal_pointer(&def); + if (pull && priv->backup->server->fdgroup) { + virStorageSourceFDTuple *fdt = NULL; + VIR_AUTOCLOSE fdcopy = -1; + + if (!(fdt = virHashLookup(priv->fds, priv->backup->server->fdgroup))) { + virReportError(VIR_ERR_INVALID_ARG, + _("file descriptor group '%1$s' was not associated with the domain"), + priv->backup->server->fdgroup); + goto endjob; + } + + if (fdt->nfds != 1) { + virReportError(VIR_ERR_INVALID_ARG, + _("file descriptor group '%1$s' must contain only 1 file descriptor for NBD server"), + priv->backup->server->fdgroup); + goto endjob; + } + + priv->backup->server->qemu_fdname = g_strdup("libvirt-backup-nbd"); + fdcopy = dup(fdt->fds[0]); + fdpass = qemuFDPassDirectNew(priv->backup->server->qemu_fdname, &fdcopy); + } + if (qemuDomainObjEnterMonitorAsync(vm, VIR_ASYNC_JOB_BACKUP) < 0) goto endjob; @@ -857,6 +881,9 @@ qemuBackupBegin(virDomainObj *vm, if (rc == 0 && tlsProps) rc = qemuMonitorAddObject(priv->mon, &tlsProps, &tlsAlias); + if (rc == 0 && fdpass) + rc = qemuFDPassDirectTransferMonitor(fdpass, priv->mon); + if (rc == 0) { if ((rc = qemuMonitorNBDServerStart(priv->mon, priv->backup->server, tlsAlias)) == 0) nbd_running = true; -- 2.49.0

From: Peter Krempa <pkrempa@redhat.com> Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- .../domainbackupxml2xmlin/backup-pull-fd.xml | 22 ++++++++++++++++++ .../domainbackupxml2xmlout/backup-pull-fd.xml | 23 +++++++++++++++++++ tests/genericxml2xmltest.c | 1 + 3 files changed, 46 insertions(+) create mode 100644 tests/domainbackupxml2xmlin/backup-pull-fd.xml create mode 100644 tests/domainbackupxml2xmlout/backup-pull-fd.xml diff --git a/tests/domainbackupxml2xmlin/backup-pull-fd.xml b/tests/domainbackupxml2xmlin/backup-pull-fd.xml new file mode 100644 index 0000000000..02ae20d665 --- /dev/null +++ b/tests/domainbackupxml2xmlin/backup-pull-fd.xml @@ -0,0 +1,22 @@ +<domainbackup mode='pull'> + <incremental>1525889631</incremental> + <server transport='fd' fdgroup='nbdfd'/> + <disks> + <disk name='vda' type='file'> + <scratch file='/path/to/file'/> + </disk> + <disk name='hda' backup='no'/> + <disk name='vdc' type='file' backupmode='full'> + <scratch file='/path/to/file'/> + </disk> + <disk name='vdd' type='file' backupmode='incremental'> + <scratch file='/path/to/file'/> + </disk> + <disk name='vde' type='file' backupmode='incremental' incremental='blah'> + <scratch file='/path/to/file'/> + </disk> + <disk name='vdf' type='file' incremental='bleh'> + <scratch file='/path/to/file'/> + </disk> + </disks> +</domainbackup> diff --git a/tests/domainbackupxml2xmlout/backup-pull-fd.xml b/tests/domainbackupxml2xmlout/backup-pull-fd.xml new file mode 100644 index 0000000000..16b9b0be50 --- /dev/null +++ b/tests/domainbackupxml2xmlout/backup-pull-fd.xml @@ -0,0 +1,23 @@ +<domainbackup mode='pull'> + <incremental>1525889631</incremental> + <server transport='fd' fdgroup='nbdfd'/> + <disks> + <disk name='vda' backup='yes' type='file' backupmode='incremental' incremental='1525889631'> + <scratch file='/path/to/file'/> + </disk> + <disk name='hda' backup='no'/> + <disk name='vdc' backup='yes' type='file' backupmode='full'> + <scratch file='/path/to/file'/> + </disk> + <disk name='vdd' backup='yes' type='file' backupmode='incremental' incremental='1525889631'> + <scratch file='/path/to/file'/> + </disk> + <disk name='vde' backup='yes' type='file' backupmode='incremental' incremental='blah'> + <scratch file='/path/to/file'/> + </disk> + <disk name='vdf' backup='yes' type='file' backupmode='incremental' incremental='bleh'> + <scratch file='/path/to/file'/> + </disk> + <disk name='vdextradisk' backup='no'/> + </disks> +</domainbackup> diff --git a/tests/genericxml2xmltest.c b/tests/genericxml2xmltest.c index b46b9515c3..dd26a8589d 100644 --- a/tests/genericxml2xmltest.c +++ b/tests/genericxml2xmltest.c @@ -245,6 +245,7 @@ mymain(void) DO_TEST_BACKUP("empty"); DO_TEST_BACKUP("backup-pull"); DO_TEST_BACKUP("backup-pull-unix"); + DO_TEST_BACKUP("backup-pull-fd"); DO_TEST_BACKUP("backup-pull-seclabel"); DO_TEST_BACKUP("backup-pull-encrypted"); DO_TEST_BACKUP("backup-push"); -- 2.49.0