Re: [PATCHv2] passt: Define backend hostname and fqdn
On Mon, Jun 23, 2025 at 5:39 PM Han Han wrote:
>
>
> On Fri, May 30, 2025 at 8:22 PM Enrique Llorente via Devel <
> devel@lists.libvirt.org> wrote:
>
>> This commit introduces a feature enhancement for configuring hostnames in
>> virtual machines (VMs) using DHCP. It adds new options to the "passt" tool
>> to set the hostname and fully qualified domain name (FQDN) for VMs. These
>> map to DHCP option 12 for the hostname and options 81 (IPv4) and 39 (IPv6)
>> for the FQDN.
>>
>> The update enables passt to dynamically assign hostnames to DHCP-aware
>> VMs. To achieve this, the commit adds two fields to the passt domain XML
>> backend. These fields allow passt to configure the hostname and FQDN for
>> the virtual machine, ensuring smooth integration with the DHCP protocol.
>>
>> This improvement is particularly valuable in environments where VMs need
>> dynamic hostname configuration, enhancing flexibility and automation in
>> virtualized network setups.
>>
>> libvirt: Integrate passt --hostname --fqdn options
>> Resolves: https://issues.redhat.com/browse/RHEL-79806
>>
>> Signed-off-by: Enrique Llorente
>> ---
>> Compared to v1 this fix the mapping between backend fqdn and hostname
>>
>> docs/formatdomain.rst | 8 +---
>> src/conf/domain_conf.c | 10 +-
>> src/conf/domain_conf.h | 2 ++
>> src/conf/schemas/domaincommon.rng | 6 ++
>> src/qemu/qemu_passt.c | 6 ++
>> tests/qemuxmlconfdata/net-user-passt.x86_64-7.2.0.xml | 2 +-
>> tests/qemuxmlconfdata/net-user-passt.x86_64-latest.xml | 2 +-
>> tests/qemuxmlconfdata/net-user-passt.xml | 2 +-
>> .../net-vhostuser-passt.x86_64-latest.xml | 2 +-
>> tests/qemuxmlconfdata/net-vhostuser-passt.xml | 2 +-
>> 10 files changed, 33 insertions(+), 9 deletions(-)
>>
>> diff --git a/docs/formatdomain.rst b/docs/formatdomain.rst
>> index 8753ee9c23..9c80aa9270 100644
>> --- a/docs/formatdomain.rst
>> +++ b/docs/formatdomain.rst
>> @@ -5372,10 +5372,12 @@ came from the host's IP.
>> There are a few other options that are configurable only for the passt
>> backend. For example, the attribute ``logFile`` can be
>> used to tell the passt process for this interface where to write its
>> -message log, and the attribute ``dev`` can tell it a
>> +message log, the attribute ``dev`` can tell it a
>> particular host interface to use when deriving the routes given to the
>> -guest for forwarding traffic upstream. Due to the design decisions of
>> -passt, when using SELinux on the host, it is recommended that the log
>> +guest for forwarding traffic upstream and the ``hostname`` and ``fqdn``
>> +will conigure the DHCP option 12 hostname and DHCP option 81 and DHCPv6
>> +option 39 fqdn attribute. Due to the design decisions of passt, when
>> using
>> +SELinux on the host, it is recommended that the log
>> file reside in the runtime directory of the user under which the passt
>> process will run, most probably ``/run/user/$UID`` (where ``$UID`` is
>> the UID of that user), e.g. ``/run/user/1000``. Be aware that libvirt
>> diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
>> index b3b0bd7329..15143f8fa2 100644
>> --- a/src/conf/domain_conf.c
>> +++ b/src/conf/domain_conf.c
>> @@ -2909,6 +2909,8 @@ virDomainNetDefFree(virDomainNetDef *def)
>> g_free(def->backend.tap);
>> g_free(def->backend.vhost);
>> g_free(def->backend.logFile);
>> +g_free(def->backend.hostname);
>> +g_free(def->backend.fqdn);
>> virDomainNetTeamingInfoFree(def->teaming);
>> g_free(def->virtPortProfile);
>> g_free(def->script);
>> @@ -9757,6 +9759,8 @@ virDomainNetBackendParseXML(xmlNodePtr node,
>> }
>>
>> def->backend.logFile = virXMLPropString(node, "logFile");
>> +def->backend.hostname = virXMLPropString(node, "hostname");
>> +def->backend.fqdn = virXMLPropString(node, "fqdn");
>>
>> if (tap)
>> def->backend.tap = virFileSanitizePath(tap);
>> @@ -20757,7 +20761,9 @@ virDomainNetBackendIsEqual(virDomainNetBackend
>> *src,
>> if (src->type != dst->type ||
>> STRNEQ_NULLABLE(src->tap, dst->tap) ||
>> STRNEQ_NULLABLE(src->vhost, dst->vhost) ||
>> -STRNEQ_NULLABLE(src->logFile, dst->logFile)) {
>> +STRNEQ_NULLABLE(src->logFile, dst->logFile) ||
>> +STRNEQ_NULLABLE(src->hostname, dst->hostname) ||
>> +STRNEQ_NULLABLE(src->fqdn, dst->fqdn)) {
>> return false;
>> }
>> return true;
>> @@ -24838,6 +24844,8 @@ virDomainNetBackendFormat(virBuffer *buf,
>> virBufferEscapeString(&attrBuf, " tap='%s'", backend->tap);
>> virBufferEscapeString(&attrBuf, " vhost='%s'", backend->vhost);
>> virBufferEscapeString(&attrBuf, " logFile='%s'", backend->logFile);
>> +virBufferEscapeString(&attrBuf, " hostname='%s'", backend->h
Re: [PATCHv2] passt: Define backend hostname and fqdn
On Fri, May 30, 2025 at 8:22 PM Enrique Llorente via Devel <
devel@lists.libvirt.org> wrote:
> This commit introduces a feature enhancement for configuring hostnames in
> virtual machines (VMs) using DHCP. It adds new options to the "passt" tool
> to set the hostname and fully qualified domain name (FQDN) for VMs. These
> map to DHCP option 12 for the hostname and options 81 (IPv4) and 39 (IPv6)
> for the FQDN.
>
> The update enables passt to dynamically assign hostnames to DHCP-aware
> VMs. To achieve this, the commit adds two fields to the passt domain XML
> backend. These fields allow passt to configure the hostname and FQDN for
> the virtual machine, ensuring smooth integration with the DHCP protocol.
>
> This improvement is particularly valuable in environments where VMs need
> dynamic hostname configuration, enhancing flexibility and automation in
> virtualized network setups.
>
> libvirt: Integrate passt --hostname --fqdn options
> Resolves: https://issues.redhat.com/browse/RHEL-79806
>
> Signed-off-by: Enrique Llorente
> ---
> Compared to v1 this fix the mapping between backend fqdn and hostname
>
> docs/formatdomain.rst | 8 +---
> src/conf/domain_conf.c | 10 +-
> src/conf/domain_conf.h | 2 ++
> src/conf/schemas/domaincommon.rng | 6 ++
> src/qemu/qemu_passt.c | 6 ++
> tests/qemuxmlconfdata/net-user-passt.x86_64-7.2.0.xml | 2 +-
> tests/qemuxmlconfdata/net-user-passt.x86_64-latest.xml | 2 +-
> tests/qemuxmlconfdata/net-user-passt.xml | 2 +-
> .../net-vhostuser-passt.x86_64-latest.xml | 2 +-
> tests/qemuxmlconfdata/net-vhostuser-passt.xml | 2 +-
> 10 files changed, 33 insertions(+), 9 deletions(-)
>
> diff --git a/docs/formatdomain.rst b/docs/formatdomain.rst
> index 8753ee9c23..9c80aa9270 100644
> --- a/docs/formatdomain.rst
> +++ b/docs/formatdomain.rst
> @@ -5372,10 +5372,12 @@ came from the host's IP.
> There are a few other options that are configurable only for the passt
> backend. For example, the attribute ``logFile`` can be
> used to tell the passt process for this interface where to write its
> -message log, and the attribute ``dev`` can tell it a
> +message log, the attribute ``dev`` can tell it a
> particular host interface to use when deriving the routes given to the
> -guest for forwarding traffic upstream. Due to the design decisions of
> -passt, when using SELinux on the host, it is recommended that the log
> +guest for forwarding traffic upstream and the ``hostname`` and ``fqdn``
> +will conigure the DHCP option 12 hostname and DHCP option 81 and DHCPv6
> +option 39 fqdn attribute. Due to the design decisions of passt, when
> using
> +SELinux on the host, it is recommended that the log
> file reside in the runtime directory of the user under which the passt
> process will run, most probably ``/run/user/$UID`` (where ``$UID`` is
> the UID of that user), e.g. ``/run/user/1000``. Be aware that libvirt
> diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
> index b3b0bd7329..15143f8fa2 100644
> --- a/src/conf/domain_conf.c
> +++ b/src/conf/domain_conf.c
> @@ -2909,6 +2909,8 @@ virDomainNetDefFree(virDomainNetDef *def)
> g_free(def->backend.tap);
> g_free(def->backend.vhost);
> g_free(def->backend.logFile);
> +g_free(def->backend.hostname);
> +g_free(def->backend.fqdn);
> virDomainNetTeamingInfoFree(def->teaming);
> g_free(def->virtPortProfile);
> g_free(def->script);
> @@ -9757,6 +9759,8 @@ virDomainNetBackendParseXML(xmlNodePtr node,
> }
>
> def->backend.logFile = virXMLPropString(node, "logFile");
> +def->backend.hostname = virXMLPropString(node, "hostname");
> +def->backend.fqdn = virXMLPropString(node, "fqdn");
>
> if (tap)
> def->backend.tap = virFileSanitizePath(tap);
> @@ -20757,7 +20761,9 @@ virDomainNetBackendIsEqual(virDomainNetBackend
> *src,
> if (src->type != dst->type ||
> STRNEQ_NULLABLE(src->tap, dst->tap) ||
> STRNEQ_NULLABLE(src->vhost, dst->vhost) ||
> -STRNEQ_NULLABLE(src->logFile, dst->logFile)) {
> +STRNEQ_NULLABLE(src->logFile, dst->logFile) ||
> +STRNEQ_NULLABLE(src->hostname, dst->hostname) ||
> +STRNEQ_NULLABLE(src->fqdn, dst->fqdn)) {
> return false;
> }
> return true;
> @@ -24838,6 +24844,8 @@ virDomainNetBackendFormat(virBuffer *buf,
> virBufferEscapeString(&attrBuf, " tap='%s'", backend->tap);
> virBufferEscapeString(&attrBuf, " vhost='%s'", backend->vhost);
> virBufferEscapeString(&attrBuf, " logFile='%s'", backend->logFile);
> +virBufferEscapeString(&attrBuf, " hostname='%s'", backend->hostname);
> +virBufferEscapeString(&attrBuf, " fqdn='%s'", backend->fqdn);
> virXMLFormatElement(buf, "backend", &attrBuf, NULL);
> }
>
> diff
Re: [PATCH v4 0/8] qemu: acpi-generic-initiator support
On 6/8/25 22:44, Andrea Righi via Devel wrote:
> = Overview =
>
> This patch set introduces support for acpi-generic-initiator devices,
> supported by QEMU [1].
>
> The acpi-generic-initiator object is required to support Multi-Instance GPU
> (MIG) configurations on NVIDIA GPUs [2]. MIG enables partitioning of GPU
> resources into multiple isolated instances, each requiring a dedicated NUMA
> node definition.
>
> = Implementation =
>
> This patch set implements the libvirt counterpart to the QEMU feature,
> enabling users to configure acpi-generic-initiator objects within libvirt
> domain XML.
>
> This includes:
> - adding XML syntax to define acpi-generic-initiator objects,
> - resolving the acpi-generic-initiator definitions into the proper QEMU
>command-line arguments,
> - ensuring compatibility with existing NUMA configuration.
>
> = Example =
>
> - Domain XML:
> ```
> ...
>
>
>
>
>
>
>
>
>
>
>
>
>
> ...
>
> ...
>
>
>
>
>function='0x0'/>
>
>
> hostdev0
> 1
>
>
> hostdev0
> 2
>
>
> hostdev0
> 3
>
>
> hostdev0
> 4
>
>
> hostdev0
> 5
>
>
> hostdev0
> 6
>
>
> hostdev0
> 7
>
>
> hostdev0
> 8
>
>
> ```
>
> - Generated QEMU command line options:
> ```
> ... /usr/bin/qemu-system-aarch64 \
> ...
> -object
> '{"qom-type":"memory-backend-ram","id":"ram-node0","size":8589934592}' \
> -numa node,nodeid=0,cpus=0-15,memdev=ram-node0 \
> -numa node,nodeid=1 \
> -numa node,nodeid=2 \
> -numa node,nodeid=3 \
> -numa node,nodeid=4 \
> -numa node,nodeid=5 \
> -numa node,nodeid=6 \
> -numa node,nodeid=7 \
> -numa node,nodeid=8 \
> ...
> -device
> '{"driver":"vfio-pci","host":"0009:01:00.0","id":"hostdev0","bus":"pci.3","addr":"0x0"}'
> ...
> -object acpi-generic-initiator,id=gi0,pci-dev=hostdev0,node=1 \
> -object acpi-generic-initiator,id=gi1,pci-dev=hostdev0,node=2 \
> -object acpi-generic-initiator,id=gi2,pci-dev=hostdev0,node=3 \
> -object acpi-generic-initiator,id=gi3,pci-dev=hostdev0,node=4 \
> -object acpi-generic-initiator,id=gi4,pci-dev=hostdev0,node=5 \
> -object acpi-generic-initiator,id=gi5,pci-dev=hostdev0,node=6 \
> -object acpi-generic-initiator,id=gi6,pci-dev=hostdev0,node=7 \
> -object acpi-generic-initiator,id=gi7,pci-dev=hostdev0,node=8
> ```
>
> = References =
>
> [1] https://lore.kernel.org/all/20231225045603.7654-2-ank...@nvidia.com/
> [2] https://www.nvidia.com/en-in/technologies/multi-instance-gpu/
>
> This patchset is also available in the following git branch:
>
> https://github.com/arighi/libvirt.git acpi-generic-initiator-v4
>
> ChangeLog v3 -> v4:
> - add acpi-generic-initiator documentation
> - refactor virDomainAcpiInitiatorDef to use info->alias and drop the name
>attribute
> - auto-generate alias for the acpi-generic-initiator devices via
>qemuAssignDeviceAliases()
> - use g_autoptr() when possible
> - add a new entry to NEWS.rst
>
> ChangeLog v2 -> v3:
> - replaced with proper types in the XML schema
> - avoid mixing g_free() and VIR_FREE()
> - use virXMLPropString() instead of looping all XML nodes
> - report proper errors with virReportError()
> - use virBufferEscapeString() to process strings passed by the user
> - fix broken formatting of function headers
> - misc coding style fixes
>
> ChangeLog v1 -> v2:
> - split parser and driver changes in separate patches
> - introduce a new qemu capability flag
> - introduce test in qemuxmlconftest
>
> Andrea Righi (8):
> schema: Introduce acpi-generic-initiator definition
> conf: Introduce acpi-generic-initiator device
> qemu: Allow to define NUMA nodes without memory or CPUs assigned
> qemu: capabilies: Introduce QEMU_CAPS_ACPI_GENERIC_INITIATOR
> qemu: Support acpi-generic-initiator
> qemu: Add test case for acpi-generic-initiator
> docs: Document acpi-generic-initiator device
> NEWS: Mention new acpi-generic-initiator device
>
> NEWS.rst | 8 ++
> docs/formatdomain.rst | 36 ++
> src/ch/ch_domain.c | 1 +
> src/conf/domain_conf.c | 138
> +
> src/conf/domain_conf.h | 14 +++
> src/conf/domain_postparse.c| 1 +
> src/conf/domain_validate.c | 37 ++
> src/conf/numa_conf.c | 3 +
> src/conf/schemas/domaincommon.rng | 14 +++
> src/conf/virconftypes.h| 2 +
> src/hyperv/hyperv_driver.c | 1 +
> src/libxl/libxl_driver.c | 6 +
> src/lxc/lxc_driver.c | 6 +
> src/qemu/qemu_alias.c | 11 ++
> src/qemu/
Re: [PATCH 5/8] qemu: Support acpi-generic-initiator
On 6/8/25 22:44, Andrea Righi via Devel wrote:
> Add support to the qemu driver to generate the proper command line for
> the acpi-generic-initiator definitions.
>
> Signed-off-by: Andrea Righi
> ---
> src/qemu/qemu_command.c | 28
> 1 file changed, 28 insertions(+)
>
> diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
> index 79cfe60b09..cedcb7e5a5 100644
> --- a/src/qemu/qemu_command.c
> +++ b/src/qemu/qemu_command.c
> @@ -10387,6 +10387,29 @@ qemuBuildPstoreCommandLine(virCommand *cmd,
> return 0;
> }
>
> +static int
> +qemuBuildAcpiInitiatorCommandLine(virCommand *cmd,
> + const virDomainAcpiInitiatorDef
> *acpiinitiator,
> + virQEMUCaps *qemuCaps)
> +{
> +g_autoptr(virJSONValue) props = NULL;
> +
> +if (!virQEMUCapsGet(qemuCaps, QEMU_CAPS_ACPI_GENERIC_INITIATOR))
> +return -1;
> +
No. Firstly, this doesn't belong here but into qemu_validate.c where a
new function must be invented that checks if domain def has an initiator
defined and if so then whether the capability is present.
Secondly, having this without any virReportError() will result in an
unknown error.
> +if (virJSONValueObjectAdd(&props,
> + "s:qom-type", "acpi-generic-initiator",
> + "s:id", acpiinitiator->info.alias,
> + "s:pci-dev", acpiinitiator->pciDev,
> + "i:node", acpiinitiator->numaNode,
> + NULL) < 0)
> +return -1;
> +
> +if (qemuBuildObjectCommandlineFromJSON(cmd, props) < 0)
> +return -1;
> +
> +return 0;
> +}
>
> static int
> qemuBuildAsyncTeardownCommandLine(virCommand *cmd,
> @@ -10741,6 +10764,11 @@ qemuBuildCommandLine(virDomainObj *vm,
> qemuBuildPstoreCommandLine(cmd, def, def->pstore, qemuCaps) < 0)
> return NULL;
>
> +for (i = 0; i < def->nacpiinitiator; i++) {
> +if (qemuBuildAcpiInitiatorCommandLine(cmd, def->acpiinitiator[i],
> qemuCaps) < 0)
> +return NULL;
> +}
> +
> if (qemuBuildAsyncTeardownCommandLine(cmd, def, qemuCaps) < 0)
> return NULL;
>
Michal
Re: [PATCH 4/8] qemu: capabilies: Introduce QEMU_CAPS_ACPI_GENERIC_INITIATOR
On 6/8/25 22:44, Andrea Righi via Devel wrote:
> This capability tracks whether QEMU supports the acpi-generic-initiator
> object type.
>
> This object has been introduced in QEMU with the commit:
> b64b7ed8bb ("qom: new object to associate device to NUMA node").
>
> Signed-off-by: Andrea Righi
> ---
> src/qemu/qemu_capabilities.c | 2 ++
> src/qemu/qemu_capabilities.h | 1 +
> tests/qemucapabilitiesdata/caps_10.0.0_aarch64.xml | 1 +
> tests/qemucapabilitiesdata/caps_10.0.0_x86_64+amdsev.xml | 1 +
> tests/qemucapabilitiesdata/caps_10.0.0_x86_64.xml| 1 +
> tests/qemucapabilitiesdata/caps_9.0.0_x86_64.xml | 1 +
> tests/qemucapabilitiesdata/caps_9.1.0_riscv64.xml| 1 +
> tests/qemucapabilitiesdata/caps_9.1.0_x86_64.xml | 1 +
> tests/qemucapabilitiesdata/caps_9.2.0_aarch64+hvf.xml| 1 +
> tests/qemucapabilitiesdata/caps_9.2.0_x86_64+amdsev.xml | 1 +
> tests/qemucapabilitiesdata/caps_9.2.0_x86_64.xml | 1 +
> 11 files changed, 12 insertions(+)
>
> diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c
> index 8ba528fc07..a8a15c56f8 100644
> --- a/src/qemu/qemu_capabilities.c
> +++ b/src/qemu/qemu_capabilities.c
> @@ -734,6 +734,7 @@ VIR_ENUM_IMPL(virQEMUCaps,
>"virtio-scsi.iothread-mapping", /*
> QEMU_CAPS_VIRTIO_SCSI_IOTHREAD_MAPPING */
>"machine.virt.highmem-mmio-size", /*
> QEMU_CAPS_MACHINE_VIRT_HIGHMEM_MMIO_SIZE */
>"bus-floppy", /* QEMU_CAPS_BUS_FLOPPY */
> + "acpi-generic-initiator", /* QEMU_CAPS_ACPI_GENERIC_INITIATOR
> */
> );
>
>
> @@ -1423,6 +1424,7 @@ struct virQEMUCapsStringFlags virQEMUCapsObjectTypes[]
> = {
> { "sev-snp-guest", QEMU_CAPS_SEV_SNP_GUEST },
> { "acpi-erst", QEMU_CAPS_DEVICE_ACPI_ERST },
> { "virtio-mem-ccw", QEMU_CAPS_DEVICE_VIRTIO_MEM_CCW },
> +{ "acpi-generic-initiator", QEMU_CAPS_ACPI_GENERIC_INITIATOR },
> };
>
>
> diff --git a/src/qemu/qemu_capabilities.h b/src/qemu/qemu_capabilities.h
> index 1e7e4faa9b..fb6a0751f7 100644
> --- a/src/qemu/qemu_capabilities.h
> +++ b/src/qemu/qemu_capabilities.h
> @@ -715,6 +715,7 @@ typedef enum { /* virQEMUCapsFlags grouping marker for
> syntax-check */
> QEMU_CAPS_VIRTIO_SCSI_IOTHREAD_MAPPING, /* virtio-scsi supports
> per-virtqueue iothread mapping */
> QEMU_CAPS_MACHINE_VIRT_HIGHMEM_MMIO_SIZE, /* -machine
> virt,highmem-mmio-size= */
> QEMU_CAPS_BUS_FLOPPY, /* floppy bus supported (isa-fdc/sysbus-fdc) */
> +QEMU_CAPS_ACPI_GENERIC_INITIATOR, /* -object acpi-generic-initiator
> command supported */
d/commmand/
>
> QEMU_CAPS_LAST /* this must always be the last item */
> } virQEMUCapsFlags;
Michal
Re: [PATCH 2/8] conf: Introduce acpi-generic-initiator device
On 6/8/25 22:44, Andrea Righi via Devel wrote:
> Introduce apci-generic-initiator device to the domain XML.
>
> Example definition:
>
>
> dev0
> 1
>
>
> This enables partitioning of PCI resources into multiple isolated
> instances, each requiring a dedicated NUMA node definition, that can be
> represented by the acpi-generic-initiator object.
>
> Signed-off-by: Andrea Righi
> ---
> src/ch/ch_domain.c| 1 +
> src/conf/domain_conf.c| 122 --
> src/conf/domain_conf.h| 5 +-
> src/conf/domain_postparse.c | 1 +
> src/conf/domain_validate.c| 37 +
> src/conf/schemas/domaincommon.rng | 7 +-
> src/hyperv/hyperv_driver.c| 1 +
> src/libxl/libxl_driver.c | 6 ++
> src/lxc/lxc_driver.c | 6 ++
> src/qemu/qemu_alias.c | 11 +++
> src/qemu/qemu_command.c | 1 +
> src/qemu/qemu_domain.c| 2 +
> src/qemu/qemu_domain_address.c| 4 +
> src/qemu/qemu_driver.c| 3 +
> src/qemu/qemu_hotplug.c | 5 ++
> src/qemu/qemu_postparse.c | 1 +
> src/qemu/qemu_validate.c | 1 +
> src/test/test_driver.c| 4 +
> 18 files changed, 205 insertions(+), 13 deletions(-)
>
> diff --git a/src/qemu/qemu_domain_address.c b/src/qemu/qemu_domain_address.c
> index e89cdee487..573bbc282c 100644
> --- a/src/qemu/qemu_domain_address.c
> +++ b/src/qemu/qemu_domain_address.c
> @@ -471,6 +471,7 @@ qemuDomainDeviceSupportZPCI(virDomainDeviceDef *device)
> case VIR_DOMAIN_DEVICE_AUDIO:
> case VIR_DOMAIN_DEVICE_CRYPTO:
> case VIR_DOMAIN_DEVICE_PSTORE:
> +case VIR_DOMAIN_DEVICE_ACPI_INITIATOR:
> break;
>
> case VIR_DOMAIN_DEVICE_NONE:
> @@ -819,6 +820,9 @@
> qemuDomainDeviceCalculatePCIConnectFlags(virDomainDeviceDef *dev,
> return pciFlags;
> }
>
> +case VIR_DOMAIN_DEVICE_ACPI_INITIATOR:
> +return pciFlags;
> +
This doesn't make much sense to me. In QEMU this is just an object, not
a device. IOW, it's not 'guest visible' and thus doesn't connect to any
bus really.
> case VIR_DOMAIN_DEVICE_MEMBALLOON:
> switch (dev->data.memballoon->model) {
> case VIR_DOMAIN_MEMBALLOON_MODEL_VIRTIO_TRANSITIONAL:
Michal
Re: [PATCH 1/8] schema: Introduce acpi-generic-initiator definition
On 6/8/25 22:44, Andrea Righi via Devel wrote: > Introduce the definition of a new acpi-generic-initiator object that can > be used to link a PCI device with multiple NUMA nodes. > > Link: https://mail.gnu.org/archive/html/qemu-arm/2024-03/msg00358.html > > Signed-off-by: Andrea Righi > --- > src/conf/domain_conf.c| 26 ++ > src/conf/domain_conf.h| 13 + > src/conf/schemas/domaincommon.rng | 19 +++ > src/conf/virconftypes.h | 2 ++ > 4 files changed, 60 insertions(+) > This could have been merged with the next patch. Especially since the next patch removes couple of lines from this patch. Michal
Re: [PATCH] util: workaround libxml2 lack of thread safe initialization
On Mon, Jun 23, 2025 at 01:16:01PM +0200, Peter Krempa wrote: > On Mon, Jun 23, 2025 at 12:02:31 +0100, Daniel P. Berrangé via Devel wrote: > > From: Daniel P. Berrangé > > > > The main XML parser code global initializer historically had a mutex > > protecting it, and more recently uses a pthread_once. The RelaxNG > > code, however, relies on three other global initializers that are > > not thread safe, just relying on setting an integer "initialized" > > flag. > > > > Calling the relevant initializers from libvirt in a protected global > > initializer will protect libvirt's own concurrent usage, however, it > > cannot protect against other libraries loaded in process that might > > be using libxml2's schema code. Fortunately: > > > > * The chances of other loaded non-libvirt code using libxml is > >relatively low > > * The chances of other loaded non-libvirt code using the schema > >validation / catalog functionality inside libxml is even > >lower > > * The chances of both libvirt and the non-libvirt usage having > >their *1st* usage of libxml2 be concurrent is tiny > > Additionaly IIUC this could be problem only when using the embedded > driver mode as we don't use libxml2 in the exported API Actually we can trigger the problem easily via the public libvirt API both locally (using test:///default) and remotely (using qemu:///system), just by using VIR_DOMAIN_DEFINE_VALIDATE concurrently. See the test program in the bug report: https://gitlab.com/-/project/192693/uploads/72900e26116909000d926de4db7e6b27/badschema.c It is unlikely something else loaded by libvirtd uses libxml2, but never say never, as we link to loads of libraries indirectly. With regards, Daniel -- |: https://berrange.com -o-https://www.flickr.com/photos/dberrange :| |: https://libvirt.org -o-https://fstop138.berrange.com :| |: https://entangle-photo.org-o-https://www.instagram.com/dberrange :|
Re: [PATCH 0/4] USB hostdev: allow addressing by port
On Mon, Jun 23, 2025 at 04:11:25PM +0200, Michal Prívozník via Devel wrote: > On 4/21/25 21:38, Maximilian Martin via Devel wrote: > > This resubmission splits up the previous patch into multiple patches and > > incorporates review comments from Michal Prívozník. > > > > Currently, only vendor/product and bus/device matching are supported for > > USB host > > devices. Neither of these provide a stable and persistent way of assigning > > a guest > > a specific host device. Vendor/product can be ambiguous. Device numbers > > change on > > every enumeration. > > > > This patch adds a bus/port matching, which allows a specific port on the > > host to be > > specified using the dotted notation found in Linux's "devpath" sysfs > > attribute. > > > > This patch is based on the previous work of Thomas Hebb: > > https://lists.libvirt.org/archives/list/devel@lists.libvirt.org/message/7U3HFUW3DGDOSF4RIBRZJINKFDYCE2ZH/ > > > > Resolves: https://gitlab.com/libvirt/libvirt/-/issues/513 > > > > Signed-off-by: Maximilian Martin > > > > Maximilian Martin (4): > > virusb test data: add devpath files for port addressing > > domain_conf, virhostdev, virusb, virusb test: add bus/port matching > > schema: add USB port attribute > > docs: add description for USB port matching > > > > docs/formatdomain.rst | 29 ++-- > > src/conf/domain_conf.c| 69 +++- > > src/conf/domain_conf.h| 1 + > > src/conf/schemas/domaincommon.rng | 11 +- > > src/hypervisor/virhostdev.c | 131 +-- > > src/libvirt_private.syms | 2 - > > src/util/virusb.c | 156 ++ > > src/util/virusb.h | 32 ++-- > > tests/virusbtest.c| 149 - > > .../sys_bus_usb/devices/1-1.5.3.1/devpath | 1 + > > .../sys_bus_usb/devices/1-1.5.3.3/devpath | 1 + > > .../sys_bus_usb/devices/1-1.5.3/devpath | 1 + > > .../sys_bus_usb/devices/1-1.5.4/devpath | 1 + > > .../sys_bus_usb/devices/1-1.5.5/devpath | 1 + > > .../sys_bus_usb/devices/1-1.5.6/devpath | 1 + > > .../sys_bus_usb/devices/1-1.5/devpath | 1 + > > .../sys_bus_usb/devices/1-1.6/devpath | 1 + > > .../sys_bus_usb/devices/1-1/devpath | 1 + > > .../sys_bus_usb/devices/2-1.2/devpath | 1 + > > .../sys_bus_usb/devices/2-1/devpath | 1 + > > .../sys_bus_usb/devices/usb1/devpath | 1 + > > .../sys_bus_usb/devices/usb2/devpath | 1 + > > .../sys_bus_usb/devices/usb3/devpath | 1 + > > .../sys_bus_usb/devices/usb4/devpath | 1 + > > 24 files changed, 351 insertions(+), 244 deletions(-) > > create mode 100644 > > tests/virusbtestdata/sys_bus_usb/devices/1-1.5.3.1/devpath > > create mode 100644 > > tests/virusbtestdata/sys_bus_usb/devices/1-1.5.3.3/devpath > > create mode 100644 tests/virusbtestdata/sys_bus_usb/devices/1-1.5.3/devpath > > create mode 100644 tests/virusbtestdata/sys_bus_usb/devices/1-1.5.4/devpath > > create mode 100644 tests/virusbtestdata/sys_bus_usb/devices/1-1.5.5/devpath > > create mode 100644 tests/virusbtestdata/sys_bus_usb/devices/1-1.5.6/devpath > > create mode 100644 tests/virusbtestdata/sys_bus_usb/devices/1-1.5/devpath > > create mode 100644 tests/virusbtestdata/sys_bus_usb/devices/1-1.6/devpath > > create mode 100644 tests/virusbtestdata/sys_bus_usb/devices/1-1/devpath > > create mode 100644 tests/virusbtestdata/sys_bus_usb/devices/2-1.2/devpath > > create mode 100644 tests/virusbtestdata/sys_bus_usb/devices/2-1/devpath > > create mode 100644 tests/virusbtestdata/sys_bus_usb/devices/usb1/devpath > > create mode 100644 tests/virusbtestdata/sys_bus_usb/devices/usb2/devpath > > create mode 100644 tests/virusbtestdata/sys_bus_usb/devices/usb3/devpath > > create mode 100644 tests/virusbtestdata/sys_bus_usb/devices/usb4/devpath > > > > I've accumulated some fixes to patch 2/4 and stored them as a fixup > commit: > > https://gitlab.com/MichalPrivoznik/libvirt/-/commit/bd6f9c823b0bbdafce4a8b7426c5763ad1d77966 > > If you're fine with suggested changes I could squash them and merge. IMHO the series is incomplete as it has added new domain XML schema without adding any new test XML files to exercise it. With regards, Daniel -- |: https://berrange.com -o-https://www.flickr.com/photos/dberrange :| |: https://libvirt.org -o-https://fstop138.berrange.com :| |: https://entangle-photo.org-o-https://www.instagram.com/dberrange :|
Re: [PATCH] util: workaround libxml2 lack of thread safe initialization
On Mon, Jun 23, 2025 at 12:02:31 +0100, Daniel P. Berrangé via Devel wrote:
> From: Daniel P. Berrangé
>
> The main XML parser code global initializer historically had a mutex
> protecting it, and more recently uses a pthread_once. The RelaxNG
> code, however, relies on three other global initializers that are
> not thread safe, just relying on setting an integer "initialized"
> flag.
>
> Calling the relevant initializers from libvirt in a protected global
> initializer will protect libvirt's own concurrent usage, however, it
> cannot protect against other libraries loaded in process that might
> be using libxml2's schema code. Fortunately:
>
> * The chances of other loaded non-libvirt code using libxml is
>relatively low
> * The chances of other loaded non-libvirt code using the schema
>validation / catalog functionality inside libxml is even
>lower
> * The chances of both libvirt and the non-libvirt usage having
>their *1st* usage of libxml2 be concurrent is tiny
Additionaly IIUC this could be problem only when using the embedded
driver mode as we don't use libxml2 in the exported API
> IOW, in practice, although our solution doesn't fully fix the thread
> safety, it is good enough.
>
> libxml2 should none the less still be fixed to make its global
> initializers be thread safe without special actions by its API
> consumers.
>
> Resolves: https://gitlab.com/libvirt/libvirt/-/issues/788
> Signed-off-by: Daniel P. Berrangé
> ---
> src/util/virxml.c | 25 +
> 1 file changed, 25 insertions(+)
>
> diff --git a/src/util/virxml.c b/src/util/virxml.c
> index 9d46e5f32f..c2d5a109dc 100644
> --- a/src/util/virxml.c
> +++ b/src/util/virxml.c
> @@ -26,6 +26,8 @@
>
> #include
> #include
> +#include
> +#include
>
> #include "virerror.h"
> #include "virxml.h"
> @@ -35,6 +37,7 @@
> #include "virstring.h"
> #include "virutil.h"
> #include "viruuid.h"
> +#include "virthread.h"
> #include "configmake.h"
>
> #define VIR_FROM_THIS VIR_FROM_XML
> @@ -50,6 +53,25 @@ struct virParserData {
> };
>
>
> +static int virXMLSchemaOnceInit(void)
Please use the preferred style of return type on it's own line.
> +{
> +if (xmlSchemaInitTypes() < 0) {
> +virReportError(VIR_ERR_INTERNAL_ERROR,
> + _("Unable to initialize libxml2 schema types"));
> +return -1;
> +}
> +if (xmlRelaxNGInitTypes() < 0) {
> +virReportError(VIR_ERR_INTERNAL_ERROR,
> + _("Unable to initialize libxml2 RelaxNG data"));
> +return -1;
> +}
> +/* No return value, it just ignores errors :-( */
> +xmlInitializeCatalog();
> +return 0;
> +}
> +
> +VIR_ONCE_GLOBAL_INIT(virXMLSchema);
> +
> static xmlXPathContextPtr
> virXMLXPathContextNew(xmlDocPtr xml)
> {
> @@ -1603,6 +1625,9 @@ virXMLValidatorInit(const char *schemafile)
> {
> g_autoptr(virXMLValidator) validator = NULL;
>
> +if (virXMLSchemaInitialize() < 0)
> +return NULL;
> +
> validator = g_new0(virXMLValidator, 1);
>
> validator->schemafile = g_strdup(schemafile);
> --
> 2.49.0
>
Reviewed-by: Peter Krempa
Re: [PATCH] util: workaround libxml2 lack of thread safe initialization
On Mon, Jun 23, 2025 at 12:02:31PM +0100, Daniel P. Berrangé wrote:
> From: Daniel P. Berrangé
>
> The main XML parser code global initializer historically had a mutex
> protecting it, and more recently uses a pthread_once. The RelaxNG
> code, however, relies on three other global initializers that are
> not thread safe, just relying on setting an integer "initialized"
> flag.
>
> Calling the relevant initializers from libvirt in a protected global
> initializer will protect libvirt's own concurrent usage, however, it
> cannot protect against other libraries loaded in process that might
> be using libxml2's schema code. Fortunately:
>
> * The chances of other loaded non-libvirt code using libxml is
>relatively low
> * The chances of other loaded non-libvirt code using the schema
>validation / catalog functionality inside libxml is even
>lower
> * The chances of both libvirt and the non-libvirt usage having
>their *1st* usage of libxml2 be concurrent is tiny
>
> IOW, in practice, although our solution doesn't fully fix the thread
> safety, it is good enough.
>
> libxml2 should none the less still be fixed to make its global
> initializers be thread safe without special actions by its API
> consumers.
>
> Resolves: https://gitlab.com/libvirt/libvirt/-/issues/788
> Signed-off-by: Daniel P. Berrangé
> ---
> src/util/virxml.c | 25 +
> 1 file changed, 25 insertions(+)
>
> diff --git a/src/util/virxml.c b/src/util/virxml.c
> index 9d46e5f32f..c2d5a109dc 100644
> --- a/src/util/virxml.c
> +++ b/src/util/virxml.c
> @@ -26,6 +26,8 @@
>
> #include
> #include
> +#include
> +#include
>
> #include "virerror.h"
> #include "virxml.h"
> @@ -35,6 +37,7 @@
> #include "virstring.h"
> #include "virutil.h"
> #include "viruuid.h"
> +#include "virthread.h"
> #include "configmake.h"
>
> #define VIR_FROM_THIS VIR_FROM_XML
> @@ -50,6 +53,25 @@ struct virParserData {
> };
>
>
> +static int virXMLSchemaOnceInit(void)
> +{
> +if (xmlSchemaInitTypes() < 0) {
> +virReportError(VIR_ERR_INTERNAL_ERROR,
> + _("Unable to initialize libxml2 schema types"));
> +return -1;
> +}
Sigh, this doesn't compile on c9s because this function was
'void' in 2.10.0 and earlier, where as now it is 'int' return
type. I'll post a v2...
With regards,
Daniel
--
|: https://berrange.com -o-https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org -o-https://fstop138.berrange.com :|
|: https://entangle-photo.org-o-https://www.instagram.com/dberrange :|
Re: [PATCH 2/4] domain_conf, virhostdev, virusb, virusb test: add bus/port matching
On 4/21/25 21:38, Maximilian Martin via Devel wrote:
> From: Maximilian Martin
>
> This patch implements USB bus/port matching. In addition
> to vendor/product and bus/device matching, bus/port
> matching is implemented. This involves additions and
> refactorings in the domain configuration, host device
> handling, and USB helpers. Also, test methods are
> refactored and extended.
>
> Signed-off-by: Maximilian Martin
> ---
> src/conf/domain_conf.c | 69 ++--
> src/conf/domain_conf.h | 1 +
> src/hypervisor/virhostdev.c | 131 +-
> src/libvirt_private.syms| 2 -
> src/util/virusb.c | 156
> src/util/virusb.h | 32
> tests/virusbtest.c | 149 --
> 7 files changed, 312 insertions(+), 228 deletions(-)
There's still a lot happening here, but see my comments below.
>
> diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
> index 542d6ade91..423cad99e5 100644
> --- a/src/conf/domain_conf.c
> +++ b/src/conf/domain_conf.c
> @@ -2670,6 +2670,15 @@
> virDomainHostdevSubsysSCSIClear(virDomainHostdevSubsysSCSI *scsisrc)
> }
> }
>
> +static void
> +virDomainHostdevSubsysUSBClear(virDomainHostdevSubsysUSB *usbsrc)
> +{
> +if (!usbsrc)
> +return;
> +
> +VIR_FREE(usbsrc->port);
> +}
> +
>
> static void
> virDomainHostdevDefClear(virDomainHostdevDef *def)
> @@ -2713,6 +2722,8 @@ virDomainHostdevDefClear(virDomainHostdevDef *def)
> g_clear_pointer(&def->source.subsys.u.pci.origstates,
> virBitmapFree);
> break;
> case VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_USB:
> +virDomainHostdevSubsysUSBClear(&def->source.subsys.u.usb);
> +break;
> case VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_MDEV:
> case VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_LAST:
> break;
> @@ -5945,13 +5956,47 @@ virDomainHostdevSubsysUSBDefParseXML(xmlNodePtr node,
> }
>
> if ((addressNode = virXPathNode("./address", ctxt))) {
> -if (virXMLPropUInt(addressNode, "bus", 0,
> - VIR_XML_PROP_REQUIRED, &usbsrc->bus) < 0)
> +bool found_device = false;
> +bool found_port = false;
Nitpick, we prefer cammelCase.
> +char *port = NULL;
> +int ret = -1;
The 'ret' name is reserved for actual return value of functions. For
storing of retvals of other functions you can use 'rc', 'rv' or anything
else.
> +
> +ret = virXMLPropUInt(addressNode, "bus", 0,
> +VIR_XML_PROP_REQUIRED, &usbsrc->bus);
> +if (ret < 0) {
> +return -1;
> +} else if (ret == 0) {
> +virReportError(VIR_ERR_INTERNAL_ERROR,
> + "%s", _("missing bus"));
> return -1;
This case can never happen. When VIR_XML_PROP_REQUIRED flag is specified
then virXMLProp*() functions return either -1 or 1.
> +}
>
> -if (virXMLPropUInt(addressNode, "device", 0,
> - VIR_XML_PROP_REQUIRED, &usbsrc->device) < 0)
> +ret = virXMLPropUInt(addressNode, "device", 0,
> +VIR_XML_PROP_NONE, &usbsrc->device);
> +if (ret < 0)
> return -1;
> +else if (ret > 0)
> +found_device = true;
> +
> +port = virXMLPropString(addressNode, "port");
> +if (port) {
> +if (*port) {
> +usbsrc->port = port;
> +found_port = true;
> +} else {
> +VIR_FREE(port);
If 'port' is declared as g_autofree then this is needless.
> +}
> +}
> +
> +if (!found_device && !found_port) {
> +virReportError(VIR_ERR_INTERNAL_ERROR,
> +"%s", _("usb address needs either device id or port"));
> +return -1;
> +} else if (found_device && found_port) {
> +virReportError(VIR_ERR_INTERNAL_ERROR,
> +"%s", _("found both device id and port in usb address
> (ambiguous setting)"));
> +return -1;
> +}
> }
>
> return 0;
> @@ -14774,8 +14819,13 @@ virDomainHostdevMatchSubsysUSB(virDomainHostdevDef
> *first,
> virDomainHostdevSubsysUSB *first_usbsrc = &first->source.subsys.u.usb;
> virDomainHostdevSubsysUSB *second_usbsrc = &second->source.subsys.u.usb;
>
> -if (first_usbsrc->bus && first_usbsrc->device) {
> -/* specified by bus location on host */
> +if (first_usbsrc->bus && first_usbsrc->port) {
> +/* specified by bus and port on host */
> +if (first_usbsrc->bus == second_usbsrc->bus &&
> +STREQ_NULLABLE(first_usbsrc->port, second_usbsrc->port))
> +return 1;
> +} else if (first_usbsrc->bus && first_usbsrc->device) {
> +/* specified by bus and device id on host */
> if (first_usbsrc->bus == second_usbsrc->bus &&
> first_usbsrc
Re: [PATCH 0/4] USB hostdev: allow addressing by port
On 4/21/25 21:38, Maximilian Martin via Devel wrote: > This resubmission splits up the previous patch into multiple patches and > incorporates review comments from Michal Prívozník. > > Currently, only vendor/product and bus/device matching are supported for USB > host > devices. Neither of these provide a stable and persistent way of assigning a > guest > a specific host device. Vendor/product can be ambiguous. Device numbers > change on > every enumeration. > > This patch adds a bus/port matching, which allows a specific port on the host > to be > specified using the dotted notation found in Linux's "devpath" sysfs > attribute. > > This patch is based on the previous work of Thomas Hebb: > https://lists.libvirt.org/archives/list/devel@lists.libvirt.org/message/7U3HFUW3DGDOSF4RIBRZJINKFDYCE2ZH/ > > Resolves: https://gitlab.com/libvirt/libvirt/-/issues/513 > > Signed-off-by: Maximilian Martin > > Maximilian Martin (4): > virusb test data: add devpath files for port addressing > domain_conf, virhostdev, virusb, virusb test: add bus/port matching > schema: add USB port attribute > docs: add description for USB port matching > > docs/formatdomain.rst | 29 ++-- > src/conf/domain_conf.c| 69 +++- > src/conf/domain_conf.h| 1 + > src/conf/schemas/domaincommon.rng | 11 +- > src/hypervisor/virhostdev.c | 131 +-- > src/libvirt_private.syms | 2 - > src/util/virusb.c | 156 ++ > src/util/virusb.h | 32 ++-- > tests/virusbtest.c| 149 - > .../sys_bus_usb/devices/1-1.5.3.1/devpath | 1 + > .../sys_bus_usb/devices/1-1.5.3.3/devpath | 1 + > .../sys_bus_usb/devices/1-1.5.3/devpath | 1 + > .../sys_bus_usb/devices/1-1.5.4/devpath | 1 + > .../sys_bus_usb/devices/1-1.5.5/devpath | 1 + > .../sys_bus_usb/devices/1-1.5.6/devpath | 1 + > .../sys_bus_usb/devices/1-1.5/devpath | 1 + > .../sys_bus_usb/devices/1-1.6/devpath | 1 + > .../sys_bus_usb/devices/1-1/devpath | 1 + > .../sys_bus_usb/devices/2-1.2/devpath | 1 + > .../sys_bus_usb/devices/2-1/devpath | 1 + > .../sys_bus_usb/devices/usb1/devpath | 1 + > .../sys_bus_usb/devices/usb2/devpath | 1 + > .../sys_bus_usb/devices/usb3/devpath | 1 + > .../sys_bus_usb/devices/usb4/devpath | 1 + > 24 files changed, 351 insertions(+), 244 deletions(-) > create mode 100644 tests/virusbtestdata/sys_bus_usb/devices/1-1.5.3.1/devpath > create mode 100644 tests/virusbtestdata/sys_bus_usb/devices/1-1.5.3.3/devpath > create mode 100644 tests/virusbtestdata/sys_bus_usb/devices/1-1.5.3/devpath > create mode 100644 tests/virusbtestdata/sys_bus_usb/devices/1-1.5.4/devpath > create mode 100644 tests/virusbtestdata/sys_bus_usb/devices/1-1.5.5/devpath > create mode 100644 tests/virusbtestdata/sys_bus_usb/devices/1-1.5.6/devpath > create mode 100644 tests/virusbtestdata/sys_bus_usb/devices/1-1.5/devpath > create mode 100644 tests/virusbtestdata/sys_bus_usb/devices/1-1.6/devpath > create mode 100644 tests/virusbtestdata/sys_bus_usb/devices/1-1/devpath > create mode 100644 tests/virusbtestdata/sys_bus_usb/devices/2-1.2/devpath > create mode 100644 tests/virusbtestdata/sys_bus_usb/devices/2-1/devpath > create mode 100644 tests/virusbtestdata/sys_bus_usb/devices/usb1/devpath > create mode 100644 tests/virusbtestdata/sys_bus_usb/devices/usb2/devpath > create mode 100644 tests/virusbtestdata/sys_bus_usb/devices/usb3/devpath > create mode 100644 tests/virusbtestdata/sys_bus_usb/devices/usb4/devpath > I've accumulated some fixes to patch 2/4 and stored them as a fixup commit: https://gitlab.com/MichalPrivoznik/libvirt/-/commit/bd6f9c823b0bbdafce4a8b7426c5763ad1d77966 If you're fine with suggested changes I could squash them and merge. Michal
Re: [libvirt PATCH v2 3/7] qemu: add IOMMU model amd
On Wed, Jun 18, 2025 at 12:46:19 +0200, Ján Tomko via Devel wrote: > From: Ján Tomko > > Introduce a new IOMMU device model 'amd', both the parser > and the formatter for QEMU because of our enum warnings. > > https://issues.redhat.com/browse/RHEL-50560 > > Signed-off-by: Ján Tomko > --- > docs/formatdomain.rst | 5 ++- > src/conf/domain_conf.c| 1 + > src/conf/domain_conf.h| 1 + > src/conf/domain_validate.c| 13 +++ > src/conf/schemas/domaincommon.rng | 1 + > src/qemu/qemu_command.c | 28 + > src/qemu/qemu_domain_address.c| 4 ++ > src/qemu/qemu_validate.c | 22 +++ > .../amd-iommu.x86_64-latest.args | 35 + > .../amd-iommu.x86_64-latest.xml | 1 + > tests/qemuxmlconfdata/amd-iommu.xml | 39 +++ > tests/qemuxmlconftest.c | 2 + > 12 files changed, 150 insertions(+), 2 deletions(-) > create mode 100644 tests/qemuxmlconfdata/amd-iommu.x86_64-latest.args > create mode 12 tests/qemuxmlconfdata/amd-iommu.x86_64-latest.xml > create mode 100644 tests/qemuxmlconfdata/amd-iommu.xml Reviewed-by: Peter Krempa
Re: [libvirt PATCH v2 6/7] conf: reject some attributes not applicable to intel IOMMU
On Wed, Jun 18, 2025 at 12:46:22 +0200, Ján Tomko via Devel wrote:
> From: Ján Tomko
>
> Signed-off-by: Ján Tomko
> ---
> src/conf/domain_validate.c | 9 +
> 1 file changed, 9 insertions(+)
>
> diff --git a/src/conf/domain_validate.c b/src/conf/domain_validate.c
> index b9a6740437..b28af7fa56 100644
> --- a/src/conf/domain_validate.c
> +++ b/src/conf/domain_validate.c
> @@ -3095,6 +3095,15 @@ virDomainIOMMUDefValidate(const virDomainIOMMUDef
> *iommu)
> break;
>
> case VIR_DOMAIN_IOMMU_MODEL_INTEL:
> +if (iommu->pt != VIR_TRISTATE_SWITCH_ABSENT ||
> +iommu->xtsup != VIR_TRISTATE_SWITCH_ABSENT) {
> +virReportError(VIR_ERR_XML_ERROR,
> + _("iommu model '%1$s' doesn't support some
> additional attributes"),
> + virDomainIOMMUModelTypeToString(iommu->model));
> +return -1;
> +}
> +break;
Same as with the 'amd' validator, the error is not very helpful.
Also this really should be squashed into 5/7.
With the above:
Reviewed-by: Peter Krempa
Re: [libvirt PATCH v2 7/7] qemu: format pt and xstup on the command line
On Wed, Jun 18, 2025 at 12:46:23 +0200, Ján Tomko via Devel wrote:
> From: Ján Tomko
>
> Signed-off-by: Ján Tomko
> ---
> src/qemu/qemu_command.c| 2 ++
> tests/qemuxmlconfdata/amd-iommu.x86_64-latest.args | 2 +-
> 2 files changed, 3 insertions(+), 1 deletion(-)
>
> diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
> index 6fae9b1f5a..b752a828ba 100644
> --- a/src/qemu/qemu_command.c
> +++ b/src/qemu/qemu_command.c
> @@ -6227,6 +6227,8 @@ qemuBuildIOMMUCommandLine(virCommand *cmd,
>"s:driver", "amd-iommu",
>"s:pci-id", iommu->info.alias,
>"S:intremap",
> qemuOnOffAuto(iommu->intremap),
> + "T:pt", iommu->pt,
> + "T:xtsup", iommu->xtsup,
>"T:device-iotlb", iommu->iotlb,
>NULL) < 0)
> return -1;
> diff --git a/tests/qemuxmlconfdata/amd-iommu.x86_64-latest.args
> b/tests/qemuxmlconfdata/amd-iommu.x86_64-latest.args
> index 36244edb3a..20d7e379e6 100644
> --- a/tests/qemuxmlconfdata/amd-iommu.x86_64-latest.args
> +++ b/tests/qemuxmlconfdata/amd-iommu.x86_64-latest.args
> @@ -27,7 +27,7 @@
> XDG_CONFIG_HOME=/var/lib/libvirt/qemu/domain--1-QEMUGuest1/.config \
> -no-shutdown \
> -boot strict=on \
> -device '{"driver":"AMDVI-PCI","id":"iommu0","bus":"pcie.0","addr":"0x1"}' \
> --device
> '{"driver":"amd-iommu","pci-id":"iommu0","intremap":"on","device-iotlb":true}'
> \
> +-device
> '{"driver":"amd-iommu","pci-id":"iommu0","intremap":"on","pt":true,"xtsup":true,"device-iotlb":true}'
> \
> -audiodev '{"id":"audio1","driver":"none"}' \
> -global ICH9-LPC.noreboot=off \
> -watchdog-action reset \
I wouldn't mind if this were squashed to 5/7, but regardless:
Reviewed-by: Peter Krempa
Re: [libvirt PATCH v2 4/7] docs: formatdomain: document intel-only IOMMU attributes
On Wed, Jun 18, 2025 at 12:46:20 +0200, Ján Tomko via Devel wrote: > From: Ján Tomko > > Signed-off-by: Ján Tomko > --- > docs/formatdomain.rst | 9 + > 1 file changed, 5 insertions(+), 4 deletions(-) IMO this patch should come before 3/7. Reviewed-by: Peter Krempa
Re: [libvirt PATCH v2 5/7] conf: add passthrough and xtsup attributes for IOMMU
On Wed, Jun 18, 2025 at 12:46:21 +0200, Ján Tomko via Devel wrote: > From: Ján Tomko > > For the newly supported AMD device. > > Signed-off-by: Ján Tomko > --- > docs/formatdomain.rst | 8 > src/conf/domain_conf.c | 30 + > src/conf/domain_conf.h | 2 ++ > src/conf/schemas/domaincommon.rng | 10 ++ > tests/qemuxmlconfdata/amd-iommu.xml | 2 +- > 5 files changed, 51 insertions(+), 1 deletion(-) Since you've explicitly forbidden unsupported attributes from 'amd' you should do the same for these with 'intel'. Reviewed-by: Peter Krempa
[PATCH v2 00/13] qemu: use 'usb-bot' device to properly emulate USB cdroms
Version 2 of this series includes changes I've requested in the review and much more that I've learned during adaptation and testing: - 'usb-bot' and 'usb-storage' are not ABI compatible for CDROMs Migrating a guest from 'usb-bot'(with cdrom) to 'usb-storage' results in I/O errors on reads with linux guest. This required adding compatibility layer - exposing the model in -xml and corresponding post-parse and migratable XML config This patchset still tries as much as possible to use the 'usb-bot' device for cdroms to fix the definition - more tests - keeping of 'usb-storage' support - XML config and documentation - cleanup of the code The necessary logic is explained in the patches as comments. Akihiko Odaki (2): qemu_capabilities: Introduce QEMU_CAPS_DEVICE_USB_BOT qemu: Replace usb-storage with usb-bot Peter Krempa (11): qemuhotplugtest: Use VIR_DOMAIN_DEF_PARSE_ABI_UPDATE for virDomainDeviceDefParse qemuxmlconftest: Test various combinations of config qemusecuritytest: Use 'disk-usb-device' case instead of 'disk-cdrom-bus-other' qemuxmlconftest: Drop 'disk-cdrom-bus-other' qemuxmlconftest: Distribute testing of 'removable' disk property qemuxmlconftest: Invoke "disk-usb-device" case also without QEMU_CAPS_DEVICE_USB_BOT and with ABI_UPDATE conf: introduce usb disk models 'usb-storage' and 'usb-bot' qemu: Fill in model of 'usb' disks to preserve ABI compatibility qemuBuildDeviceAddresDriveProps: Prepare for 'drive' address for usb-bot disks qemu: monitor: Introduce 'qemuMonitorSetUSBDiskAttached' qemuxmlconftest: Prepare for proper testing in 'disk-cdrom-usb-empty' docs/formatdomain.rst | 23 +++- src/conf/domain_conf.c| 2 + src/conf/domain_conf.h| 3 + src/conf/schemas/domaincommon.rng | 2 + src/qemu/qemu_alias.c | 20 ++- src/qemu/qemu_capabilities.c | 7 +- src/qemu/qemu_capabilities.h | 3 + src/qemu/qemu_command.c | 101 -- src/qemu/qemu_command.h | 5 + src/qemu/qemu_domain.c| 21 +++ src/qemu/qemu_domain_address.c| 2 + src/qemu/qemu_hotplug.c | 18 +++ src/qemu/qemu_monitor.c | 12 ++ src/qemu/qemu_monitor.h | 3 + src/qemu/qemu_monitor_json.c | 20 +++ src/qemu/qemu_monitor_json.h | 5 + src/qemu/qemu_postparse.c | 49 ++- src/qemu/qemu_postparse.h | 4 +- src/qemu/qemu_validate.c | 37 +- tests/qemublocktest.c | 13 +- .../caps_10.0.0_aarch64.xml | 1 + .../caps_10.0.0_ppc64.xml | 1 + .../caps_10.0.0_s390x.xml | 1 + .../caps_10.0.0_x86_64+amdsev.xml | 1 + .../caps_10.0.0_x86_64.xml| 1 + .../qemucapabilitiesdata/caps_6.2.0_ppc64.xml | 1 + .../caps_6.2.0_x86_64.xml | 1 + .../qemucapabilitiesdata/caps_7.0.0_ppc64.xml | 1 + .../caps_7.0.0_x86_64.xml | 1 + .../qemucapabilitiesdata/caps_7.1.0_ppc64.xml | 1 + .../caps_7.1.0_x86_64.xml | 1 + tests/qemucapabilitiesdata/caps_7.2.0_ppc.xml | 1 + .../caps_7.2.0_x86_64+hvf.xml | 1 + .../caps_7.2.0_x86_64.xml | 1 + .../caps_8.0.0_x86_64.xml | 1 + .../qemucapabilitiesdata/caps_8.1.0_s390x.xml | 1 + .../caps_8.1.0_x86_64.xml | 1 + .../caps_8.2.0_aarch64.xml| 1 + .../caps_8.2.0_armv7l.xml | 1 + .../caps_8.2.0_loongarch64.xml| 1 + .../qemucapabilitiesdata/caps_8.2.0_s390x.xml | 1 + .../caps_8.2.0_x86_64.xml | 1 + .../caps_9.0.0_x86_64.xml | 1 + .../caps_9.1.0_riscv64.xml| 1 + .../qemucapabilitiesdata/caps_9.1.0_s390x.xml | 1 + .../caps_9.1.0_x86_64.xml | 1 + .../caps_9.2.0_aarch64+hvf.xml| 1 + .../qemucapabilitiesdata/caps_9.2.0_s390x.xml | 1 + .../caps_9.2.0_x86_64+amdsev.xml | 1 + .../caps_9.2.0_x86_64.xml | 1 + tests/qemuhotplugtest.c | 8 +- .../qemuhotplug-base-live+cdrom-usb.xml | 2 +- .../qemuhotplug-base-live+disk-usb.xml| 2 +- tests/qemusecuritytest.c | 2 +- .../disk-cache.x86_64-latest.xml | 2 +- .../qemuxmlconfdata/disk-cdrom-bus-other.xml | 30 - ...m-usb-empty.x86_64-latest.abi-update.args} | 4 +- ...om-usb-empty.x86_64-latest.abi-update.xml} | 6 +- .../disk-device-removable.x86_64-latest.args | 40 -- .../qemuxmlconfdata/disk-devi
[PATCH] util: workaround libxml2 lack of thread safe initialization
From: Daniel P. Berrangé
The main XML parser code global initializer historically had a mutex
protecting it, and more recently uses a pthread_once. The RelaxNG
code, however, relies on three other global initializers that are
not thread safe, just relying on setting an integer "initialized"
flag.
Calling the relevant initializers from libvirt in a protected global
initializer will protect libvirt's own concurrent usage, however, it
cannot protect against other libraries loaded in process that might
be using libxml2's schema code. Fortunately:
* The chances of other loaded non-libvirt code using libxml is
relatively low
* The chances of other loaded non-libvirt code using the schema
validation / catalog functionality inside libxml is even
lower
* The chances of both libvirt and the non-libvirt usage having
their *1st* usage of libxml2 be concurrent is tiny
IOW, in practice, although our solution doesn't fully fix the thread
safety, it is good enough.
libxml2 should none the less still be fixed to make its global
initializers be thread safe without special actions by its API
consumers.
Resolves: https://gitlab.com/libvirt/libvirt/-/issues/788
Signed-off-by: Daniel P. Berrangé
---
src/util/virxml.c | 25 +
1 file changed, 25 insertions(+)
diff --git a/src/util/virxml.c b/src/util/virxml.c
index 9d46e5f32f..c2d5a109dc 100644
--- a/src/util/virxml.c
+++ b/src/util/virxml.c
@@ -26,6 +26,8 @@
#include
#include
+#include
+#include
#include "virerror.h"
#include "virxml.h"
@@ -35,6 +37,7 @@
#include "virstring.h"
#include "virutil.h"
#include "viruuid.h"
+#include "virthread.h"
#include "configmake.h"
#define VIR_FROM_THIS VIR_FROM_XML
@@ -50,6 +53,25 @@ struct virParserData {
};
+static int virXMLSchemaOnceInit(void)
+{
+if (xmlSchemaInitTypes() < 0) {
+virReportError(VIR_ERR_INTERNAL_ERROR,
+ _("Unable to initialize libxml2 schema types"));
+return -1;
+}
+if (xmlRelaxNGInitTypes() < 0) {
+virReportError(VIR_ERR_INTERNAL_ERROR,
+ _("Unable to initialize libxml2 RelaxNG data"));
+return -1;
+}
+/* No return value, it just ignores errors :-( */
+xmlInitializeCatalog();
+return 0;
+}
+
+VIR_ONCE_GLOBAL_INIT(virXMLSchema);
+
static xmlXPathContextPtr
virXMLXPathContextNew(xmlDocPtr xml)
{
@@ -1603,6 +1625,9 @@ virXMLValidatorInit(const char *schemafile)
{
g_autoptr(virXMLValidator) validator = NULL;
+if (virXMLSchemaInitialize() < 0)
+return NULL;
+
validator = g_new0(virXMLValidator, 1);
validator->schemafile = g_strdup(schemafile);
--
2.49.0
Re: [PATCH 2/4] domain_conf, virhostdev, virusb, virusb test: add bus/port matching
On Mon, Jun 23, 2025 at 04:11:30PM +0200, Michal Prívozník via Devel wrote:
> On 4/21/25 21:38, Maximilian Martin via Devel wrote:
> > From: Maximilian Martin
> >
> > This patch implements USB bus/port matching. In addition
> > to vendor/product and bus/device matching, bus/port
> > matching is implemented. This involves additions and
> > refactorings in the domain configuration, host device
> > handling, and USB helpers. Also, test methods are
> > refactored and extended.
> >
> > Signed-off-by: Maximilian Martin
> > ---
> > src/conf/domain_conf.c | 69 ++--
> > src/conf/domain_conf.h | 1 +
> > src/hypervisor/virhostdev.c | 131 +-
> > src/libvirt_private.syms| 2 -
> > src/util/virusb.c | 156
> > src/util/virusb.h | 32
> > tests/virusbtest.c | 149 --
> > 7 files changed, 312 insertions(+), 228 deletions(-)
>
> There's still a lot happening here, but see my comments below.
>
> >
> > diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
> > index 542d6ade91..423cad99e5 100644
> > --- a/src/conf/domain_conf.c
> > +++ b/src/conf/domain_conf.c
> > @@ -2670,6 +2670,15 @@
> > virDomainHostdevSubsysSCSIClear(virDomainHostdevSubsysSCSI *scsisrc)
> > }
> > }
> >
> > +static void
> > +virDomainHostdevSubsysUSBClear(virDomainHostdevSubsysUSB *usbsrc)
> > +{
> > +if (!usbsrc)
> > +return;
> > +
> > +VIR_FREE(usbsrc->port);
> > +}
> > +
> >
> > static void
> > virDomainHostdevDefClear(virDomainHostdevDef *def)
> > @@ -2713,6 +2722,8 @@ virDomainHostdevDefClear(virDomainHostdevDef *def)
> > g_clear_pointer(&def->source.subsys.u.pci.origstates,
> > virBitmapFree);
> > break;
> > case VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_USB:
> > +virDomainHostdevSubsysUSBClear(&def->source.subsys.u.usb);
> > +break;
> > case VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_MDEV:
> > case VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_LAST:
> > break;
> > @@ -5945,13 +5956,47 @@ virDomainHostdevSubsysUSBDefParseXML(xmlNodePtr
> > node,
> > }
> >
> > if ((addressNode = virXPathNode("./address", ctxt))) {
> > -if (virXMLPropUInt(addressNode, "bus", 0,
> > - VIR_XML_PROP_REQUIRED, &usbsrc->bus) < 0)
> > +bool found_device = false;
> > +bool found_port = false;
>
> Nitpick, we prefer cammelCase.
>
> > +char *port = NULL;
> > +int ret = -1;
>
> The 'ret' name is reserved for actual return value of functions. For
> storing of retvals of other functions you can use 'rc', 'rv' or anything
> else.
>
> > +
> > +ret = virXMLPropUInt(addressNode, "bus", 0,
> > +VIR_XML_PROP_REQUIRED, &usbsrc->bus);
> > +if (ret < 0) {
> > +return -1;
> > +} else if (ret == 0) {
> > +virReportError(VIR_ERR_INTERNAL_ERROR,
> > + "%s", _("missing bus"));
> > return -1;
>
> This case can never happen. When VIR_XML_PROP_REQUIRED flag is specified
> then virXMLProp*() functions return either -1 or 1.
>
> > +}
> >
> > -if (virXMLPropUInt(addressNode, "device", 0,
> > - VIR_XML_PROP_REQUIRED, &usbsrc->device) < 0)
> > +ret = virXMLPropUInt(addressNode, "device", 0,
> > +VIR_XML_PROP_NONE, &usbsrc->device);
> > +if (ret < 0)
> > return -1;
> > +else if (ret > 0)
> > +found_device = true;
> > +
> > +port = virXMLPropString(addressNode, "port");
> > +if (port) {
> > +if (*port) {
> > +usbsrc->port = port;
> > +found_port = true;
> > +} else {
> > +VIR_FREE(port);
>
> If 'port' is declared as g_autofree then this is needless.
>
> > +}
> > +}
> > +
> > +if (!found_device && !found_port) {
> > +virReportError(VIR_ERR_INTERNAL_ERROR,
> > +"%s", _("usb address needs either device id or port"));
> > +return -1;
> > +} else if (found_device && found_port) {
> > +virReportError(VIR_ERR_INTERNAL_ERROR,
> > +"%s", _("found both device id and port in usb address
> > (ambiguous setting)"));
> > +return -1;
> > +}
> > }
> >
> > return 0;
> > @@ -14774,8 +14819,13 @@ virDomainHostdevMatchSubsysUSB(virDomainHostdevDef
> > *first,
> > virDomainHostdevSubsysUSB *first_usbsrc = &first->source.subsys.u.usb;
> > virDomainHostdevSubsysUSB *second_usbsrc =
> > &second->source.subsys.u.usb;
> >
> > -if (first_usbsrc->bus && first_usbsrc->device) {
> > -/* specified by bus location on host */
> > +if (first_usbsrc->bus && first_usbsrc->port) {
> > +/* specified by bus and port on host */
> > +
Re: [PATCH 0/4] USB hostdev: allow addressing by port
On Mon, Apr 21, 2025 at 09:38:34PM +0200, Maximilian Martin via Devel wrote: > This resubmission splits up the previous patch into multiple patches and > incorporates review comments from Michal Prívozník. > > Currently, only vendor/product and bus/device matching are supported for USB > host > devices. Neither of these provide a stable and persistent way of assigning a > guest > a specific host device. Vendor/product can be ambiguous. Device numbers > change on > every enumeration. > > This patch adds a bus/port matching, which allows a specific port on the host > to be > specified using the dotted notation found in Linux's "devpath" sysfs > attribute. In terms of our API, we're expecting people to use the 'node device' APIs to identify what devices are available on the host and their attributes, rather than queryin Linux directly. This gives us a platform independant usage model, which also works when the mgmt app has no login to the virt host. Currently for USB devices we report bus, dev, vendor & product, but don't report the port: # virsh nodedev-dumpxml usb_1_10_4 usb_1_10_4 /sys/devices/pci:00/:00:14.0/usb1/1-10/1-10.4 /dev/bus/usb/001/004 usb_1_10 usb 1 4 Dell Integrated Hub Tascam we need to add 'port' to this xml document under the 'usb_device' capability. With regards, Daniel -- |: https://berrange.com -o-https://www.flickr.com/photos/dberrange :| |: https://libvirt.org -o-https://fstop138.berrange.com :| |: https://entangle-photo.org-o-https://www.instagram.com/dberrange :|
Re: [libvirt PATCH v2 2/7] qemu: introduce QEMU_CAPS_PCI_ID
On Wed, Jun 18, 2025 at 12:46:18 +0200, Ján Tomko via Devel wrote: > From: Ján Tomko > > Introduced by QEMU commit f864a3235ea1d1d714b3cde2d9a810ea6344a7b5 > the presence of this attribute allows libvirt to specify the alias > of the AMDVI-PCI device explicitly. > > (It was implicit before the introduction of this attribute) > > Signed-off-by: Ján Tomko > --- > src/qemu/qemu_capabilities.c | 8 ++ > src/qemu/qemu_capabilities.h | 1 + > .../caps_10.0.0_x86_64+amdsev.replies | 102 +++-- > .../caps_10.0.0_x86_64.replies| 106 -- > .../caps_10.0.0_x86_64.xml| 1 + > .../caps_6.2.0_x86_64.replies | 102 +++-- > .../caps_7.0.0_x86_64.replies | 80 +++-- > .../caps_7.1.0_x86_64.replies | 102 +++-- > .../caps_7.2.0_x86_64+hvf.replies | 102 +++-- > .../caps_7.2.0_x86_64.replies | 102 +++-- > .../caps_8.0.0_x86_64.replies | 80 +++-- > .../caps_8.1.0_x86_64.replies | 102 +++-- > .../caps_8.2.0_x86_64.replies | 102 +++-- > .../caps_9.0.0_x86_64.replies | 80 +++-- > .../caps_9.1.0_x86_64.replies | 102 +++-- > .../caps_9.2.0_x86_64+amdsev.replies | 102 +++-- > .../caps_9.2.0_x86_64.replies | 102 +++-- > 17 files changed, 928 insertions(+), 448 deletions(-) Reviewed-by: Peter Krempa
Re: [libvirt PATCH v2 1/7] qemu: introduce QEMU_CAPS_AMD_IOMMU
On Wed, Jun 18, 2025 at 12:46:17 +0200, Ján Tomko via Devel wrote: > From: Ján Tomko > > Check for the presence of the amd-iommu device, so we can conditionalize > probing for its properties. > > Signed-off-by: Ján Tomko > --- > src/qemu/qemu_capabilities.c | 4 > src/qemu/qemu_capabilities.h | 3 +++ > tests/qemucapabilitiesdata/caps_10.0.0_x86_64+amdsev.xml | 1 + > tests/qemucapabilitiesdata/caps_10.0.0_x86_64.xml| 1 + > tests/qemucapabilitiesdata/caps_6.2.0_x86_64.xml | 1 + > tests/qemucapabilitiesdata/caps_7.0.0_x86_64.xml | 1 + > tests/qemucapabilitiesdata/caps_7.1.0_x86_64.xml | 1 + > tests/qemucapabilitiesdata/caps_7.2.0_x86_64+hvf.xml | 1 + > tests/qemucapabilitiesdata/caps_7.2.0_x86_64.xml | 1 + > tests/qemucapabilitiesdata/caps_8.0.0_x86_64.xml | 1 + > tests/qemucapabilitiesdata/caps_8.1.0_x86_64.xml | 1 + > tests/qemucapabilitiesdata/caps_8.2.0_x86_64.xml | 1 + > tests/qemucapabilitiesdata/caps_9.0.0_x86_64.xml | 1 + > tests/qemucapabilitiesdata/caps_9.1.0_x86_64.xml | 1 + > tests/qemucapabilitiesdata/caps_9.2.0_x86_64+amdsev.xml | 1 + > tests/qemucapabilitiesdata/caps_9.2.0_x86_64.xml | 1 + > 16 files changed, 21 insertions(+) Reviewed-by: Peter Krempa
[PATCH v2 09/13] qemu: Fill in model of 'usb' disks to preserve ABI compatibility
From: Peter Krempa
While 'usb-bot' and 'usb-storage' are ABI and migration compatible for
disks it's not the case for cdroms. When migrating from a new config
using 'usb-bot' to an older daemon which would use 'usb-storage' the
guest os will get I/O errors.
Thus we must properly fill in models for 'usb' disks so that cdroms can
be handled properly.
When parsing XML fill in the models and drop the appropriate models when
formatting migratable XML.
The logic is explained in comments in the code.
Signed-off-by: Peter Krempa
---
src/qemu/qemu_domain.c| 21
src/qemu/qemu_postparse.c | 49 ++-
src/qemu/qemu_postparse.h | 4 +-
tests/qemublocktest.c | 13 +++--
.../qemuhotplug-base-live+cdrom-usb.xml | 2 +-
.../qemuhotplug-base-live+disk-usb.xml| 2 +-
.../disk-cache.x86_64-latest.xml | 2 +-
.../disk-usb-device-model.x86_64-latest.xml | 4 +-
...test.QEMU_CAPS_DEVICE_USB_BOT-disabled.xml | 24 -
...date.QEMU_CAPS_DEVICE_USB_BOT-disabled.xml | 24 -
...sk-usb-device.x86_64-latest.abi-update.xml | 24 -
.../disk-usb-device.x86_64-latest.xml | 24 -
12 files changed, 132 insertions(+), 61 deletions(-)
diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c
index ace42b516a..6e147563f3 100644
--- a/src/qemu/qemu_domain.c
+++ b/src/qemu/qemu_domain.c
@@ -5342,6 +5342,27 @@ qemuDomainDefFormatBufInternal(virQEMUDriver *driver,
}
}
+for (i = 0; i < def->ndisks; i++) {
+virDomainDiskDef *disk = def->disks[i];
+
+/* The 'mode' property for USB disks was introduced long after USB
+ * disks to allow switching between 'usb-storage' and 'usb-bot'
+ * device. Despite sharing identical implementation 'usb-bot'
allows
+ * proper configuration of USB cdroms. Unfortunately it is not ABI
+ * compatible.
+ *
+ * To preserve migration to older daemons we can strip the model to
+ * the default if:
+ * - it's a normal disk (not cdrom) as both are identical
+ * - for a usb-cdrom strip the model if it's not 'usb-bot' as that
+ * was the old configuration
+ */
+if (disk->bus == VIR_DOMAIN_DISK_BUS_USB &&
+(disk->model == VIR_DOMAIN_DISK_MODEL_USB_STORAGE ||
+ disk->device == VIR_DOMAIN_DISK_DEVICE_DISK))
+disk->model = VIR_DOMAIN_DISK_MODEL_DEFAULT;
+}
+
/* Replace the CPU definition updated according to QEMU with the one
* used for starting the domain. The updated def will be sent
* separately for backward compatibility.
diff --git a/src/qemu/qemu_postparse.c b/src/qemu/qemu_postparse.c
index 8150dffac6..7db378c5ce 100644
--- a/src/qemu/qemu_postparse.c
+++ b/src/qemu/qemu_postparse.c
@@ -202,7 +202,8 @@
qemuDomainDeviceDiskDefPostParseRestoreSecAlias(virDomainDiskDef *disk,
int
qemuDomainDeviceDiskDefPostParse(virDomainDiskDef *disk,
- unsigned int parseFlags)
+ unsigned int parseFlags,
+ virQEMUCaps *qemuCaps)
{
virStorageSource *n;
@@ -220,6 +221,50 @@ qemuDomainDeviceDiskDefPostParse(virDomainDiskDef *disk,
disk->mirror->format == VIR_STORAGE_FILE_NONE)
disk->mirror->format = VIR_STORAGE_FILE_RAW;
+/* default USB disk model:
+ *
+ * Historically we didn't use model for USB disks. It became necessary once
+ * it turned out that 'usb-storage' doesn't properly expose CDROM devices
+ * with -blockdev. Additionally 'usb-bot' which does properly handle them,
+ * while having identical implementation in qemu and driver in guest, are
+ * not in fact ABI compatible. Thus the logic is as follows:
+ *
+ * If ABI update is not allowed:
+ * - use 'usb-storage' for either (unless only 'usb-bot' is supported)
+ *
+ * If ABI update is possible
+ * - for VIR_DOMAIN_DISK_DEVICE_DISK use 'usb-storage' as it doesn't matter
+ *(it is identical with 'usb-bot' ABI wise)
+ * - for VIR_DOMAIN_DISK_DEVICE_CDROM use 'usb-bot' if available
+ *(as it properly exposes cdrom)
+ *
+ * When formatting migratable XML the code strips 'usb-storage' to preserve
+ * migration to older daemons. If a new definition with 'usb-bot' cdrom is
+ * created via new start or hotplug it will fail migrating. Users must
+ * explicitly set the broken config in XML or unplug the device.
+ */
+if (qemuCaps &&
+disk->bus == VIR_DOMAIN_DISK_BUS_USB &&
+disk->model == VIR_DOMAIN_DISK_MODEL_DEFAULT) {
+
+if (disk->device == VIR_DOMAIN_DISK_DEVICE_CDROM &&
+parseFlags & VIR_DOMAIN_DEF_PARSE_ABI_UPDATE) {
+
+if (virQEMUCapsGet(qemuCaps, QEMU_
[PATCH v2 05/13] qemuxmlconftest: Distribute testing of 'removable' disk property
From: Peter Krempa
The 'removable' property is tested for 'usb' and 'scsi' disks. The test
case for 'usb' disks already has another test case for this, so add
testing of 'removable' SCSI disks into the 'disk-scsi' case and remove
the 'disk-device-removable' case completely.
Signed-off-by: Peter Krempa
---
.../disk-device-removable.x86_64-latest.args | 40 --
.../disk-device-removable.x86_64-latest.xml | 54 ---
.../qemuxmlconfdata/disk-device-removable.xml | 32 ---
.../disk-scsi.x86_64-latest.args | 2 +-
tests/qemuxmlconfdata/disk-scsi.xml | 2 +-
tests/qemuxmlconftest.c | 1 -
6 files changed, 2 insertions(+), 129 deletions(-)
delete mode 100644
tests/qemuxmlconfdata/disk-device-removable.x86_64-latest.args
delete mode 100644
tests/qemuxmlconfdata/disk-device-removable.x86_64-latest.xml
delete mode 100644 tests/qemuxmlconfdata/disk-device-removable.xml
diff --git a/tests/qemuxmlconfdata/disk-device-removable.x86_64-latest.args
b/tests/qemuxmlconfdata/disk-device-removable.x86_64-latest.args
deleted file mode 100644
index e0701f4bd2..00
--- a/tests/qemuxmlconfdata/disk-device-removable.x86_64-latest.args
+++ /dev/null
@@ -1,40 +0,0 @@
-LC_ALL=C \
-PATH=/bin \
-HOME=/var/lib/libvirt/qemu/domain--1-QEMUGuest1 \
-USER=test \
-LOGNAME=test \
-XDG_DATA_HOME=/var/lib/libvirt/qemu/domain--1-QEMUGuest1/.local/share \
-XDG_CACHE_HOME=/var/lib/libvirt/qemu/domain--1-QEMUGuest1/.cache \
-XDG_CONFIG_HOME=/var/lib/libvirt/qemu/domain--1-QEMUGuest1/.config \
-/usr/bin/qemu-system-x86_64 \
--name guest=QEMUGuest1,debug-threads=on \
--S \
--object
'{"qom-type":"secret","id":"masterKey0","format":"raw","file":"/var/lib/libvirt/qemu/domain--1-QEMUGuest1/master-key.aes"}'
\
--machine pc,usb=off,dump-guest-core=off,memory-backend=pc.ram,acpi=off \
--accel tcg \
--cpu qemu64 \
--m size=219136k \
--object '{"qom-type":"memory-backend-ram","id":"pc.ram","size":224395264}' \
--overcommit mem-lock=off \
--smp 1,sockets=1,cores=1,threads=1 \
--uuid c7a5fdbd-edaf-9455-926a-d65c16db1809 \
--display none \
--no-user-config \
--nodefaults \
--chardev socket,id=charmonitor,fd=1729,server=on,wait=off \
--mon chardev=charmonitor,id=monitor,mode=control \
--rtc base=utc \
--no-shutdown \
--boot strict=on \
--device
'{"driver":"piix3-usb-uhci","id":"usb","bus":"pci.0","addr":"0x1.0x2"}' \
--device '{"driver":"virtio-scsi-pci","id":"scsi0","bus":"pci.0","addr":"0x2"}'
\
--blockdev
'{"driver":"host_device","filename":"/dev/HostVG/QEMUGuest1","node-name":"libvirt-3-storage","read-only":false}'
\
--device
'{"driver":"ide-hd","bus":"ide.0","unit":0,"drive":"libvirt-3-storage","id":"ide0-0-0","bootindex":1}'
\
--blockdev
'{"driver":"file","filename":"/tmp/usbdisk.img","node-name":"libvirt-2-storage","read-only":false}'
\
--device
'{"driver":"usb-storage","bus":"usb.0","port":"1","drive":"libvirt-2-storage","id":"usb-disk0","removable":true}'
\
--blockdev
'{"driver":"file","filename":"/tmp/scsidisk.img","node-name":"libvirt-1-storage","read-only":false}'
\
--device
'{"driver":"scsi-hd","bus":"scsi0.0","channel":0,"scsi-id":0,"lun":1,"device_id":"drive-scsi0-0-0-1","drive":"libvirt-1-storage","id":"scsi0-0-0-1","removable":true}'
\
--audiodev '{"id":"audio1","driver":"none"}' \
--device
'{"driver":"virtio-balloon-pci","id":"balloon0","bus":"pci.0","addr":"0x3"}' \
--sandbox
on,obsolete=deny,elevateprivileges=deny,spawn=deny,resourcecontrol=deny \
--msg timestamp=on
diff --git a/tests/qemuxmlconfdata/disk-device-removable.x86_64-latest.xml
b/tests/qemuxmlconfdata/disk-device-removable.x86_64-latest.xml
deleted file mode 100644
index 2fd3284dee..00
--- a/tests/qemuxmlconfdata/disk-device-removable.x86_64-latest.xml
+++ /dev/null
@@ -1,54 +0,0 @@
-
- QEMUGuest1
- c7a5fdbd-edaf-9455-926a-d65c16db1809
- 219136
- 219136
- 1
-
-hvm
-
-
-
-qemu64
-
-
- destroy
- restart
- destroy
-
-/usr/bin/qemu-system-x86_64
-
-
-
-
-
-
-
-
-
-
-
-
-
-
-
-
-
-
-
-
-
-
-
-
-
-
-
-
-
-
-
-
-
-
-
diff --git a/tests/qemuxmlconfdata/disk-device-removable.xml
b/tests/qemuxmlconfdata/disk-device-removable.xml
deleted file mode 100644
index 9400c84863..00
--- a/tests/qemuxmlconfdata/disk-device-removable.xml
+++ /dev/null
@@ -1,32 +0,0 @@
-
- QEMUGuest1
- c7a5fdbd-edaf-9455-926a-d65c16db1809
- 219136
- 219136
- 1
-
-hvm
-
-
-
- destroy
- restart
- destroy
-
-/usr/bin/qemu-system-x86_64
-
-
-
-
-
-
-
-
-
-
-
-
-
-
-
-
diff --git a/tests/qemuxmlconfdata/disk-scsi.x86_64-latest.args
b/tests/qemuxmlconfdata/disk-scsi.x86_64-latest.args
index 1b8150f4d4..cd07cd2d22 100644
--- a/tests/qemuxmlconfdata/disk-scsi.x86_64-latest.args
+++ b/tests/qemuxmlc
[PATCH v2 02/13] qemuxmlconftest: Test various combinations of config
From: Peter Krempa
Add multiple USB disks to the definition testing a matrix of 'disk' and
'cdrom' elements with user-aliases, 'serial' and 'removable'
properties configured.
This patch also removes the 'ide' disk which is not related to what
we're testing here.
Signed-off-by: Peter Krempa
---
.../disk-usb-device.x86_64-latest.args| 30 ++-
.../disk-usb-device.x86_64-latest.xml | 80 ---
tests/qemuxmlconfdata/disk-usb-device.xml | 66 +--
3 files changed, 158 insertions(+), 18 deletions(-)
diff --git a/tests/qemuxmlconfdata/disk-usb-device.x86_64-latest.args
b/tests/qemuxmlconfdata/disk-usb-device.x86_64-latest.args
index 0fd7e755b1..079dfe5d99 100644
--- a/tests/qemuxmlconfdata/disk-usb-device.x86_64-latest.args
+++ b/tests/qemuxmlconfdata/disk-usb-device.x86_64-latest.args
@@ -27,10 +27,32 @@
XDG_CONFIG_HOME=/var/lib/libvirt/qemu/domain--1-QEMUGuest1/.config \
-no-shutdown \
-boot strict=on \
-device
'{"driver":"piix3-usb-uhci","id":"usb","bus":"pci.0","addr":"0x1.0x2"}' \
--blockdev
'{"driver":"host_device","filename":"/dev/HostVG/QEMUGuest1","node-name":"libvirt-2-storage","read-only":false}'
\
--device
'{"driver":"ide-hd","bus":"ide.0","unit":0,"drive":"libvirt-2-storage","id":"ide0-0-0","bootindex":1}'
\
--blockdev
'{"driver":"file","filename":"/tmp/usbdisk.img","node-name":"libvirt-1-storage","read-only":false}'
\
--device
'{"driver":"usb-storage","bus":"usb.0","port":"1","drive":"libvirt-1-storage","id":"usb-disk0","removable":false}'
\
+-device '{"driver":"usb-hub","id":"hub0","bus":"usb.0","port":"1"}' \
+-device '{"driver":"usb-hub","id":"hub1","bus":"usb.0","port":"2"}' \
+-blockdev
'{"driver":"file","filename":"/tmp/img1","node-name":"libvirt-12-storage","read-only":false}'
\
+-device
'{"driver":"usb-storage","bus":"usb.0","port":"1.1","drive":"libvirt-12-storage","id":"usb-disk0","bootindex":1,"removable":false}'
\
+-blockdev
'{"driver":"file","filename":"/tmp/img2","node-name":"libvirt-11-storage","read-only":true}'
\
+-device
'{"driver":"usb-storage","bus":"usb.0","port":"1.2","drive":"libvirt-11-storage","id":"usb-disk1","removable":false}'
\
+-blockdev
'{"driver":"file","filename":"/tmp/img3","node-name":"libvirt-10-storage","read-only":false}'
\
+-device
'{"driver":"usb-storage","bus":"usb.0","port":"1.3","drive":"libvirt-10-storage","id":"usb-disk2","removable":false,"serial":"testserial1"}'
\
+-blockdev
'{"driver":"file","filename":"/tmp/img4","node-name":"libvirt-9-storage","read-only":true}'
\
+-device
'{"driver":"usb-storage","bus":"usb.0","port":"1.4","drive":"libvirt-9-storage","id":"usb-disk3","removable":false,"serial":"testserial2"}'
\
+-blockdev
'{"driver":"file","filename":"/tmp/img5","node-name":"libvirt-8-storage","read-only":false}'
\
+-device
'{"driver":"usb-storage","bus":"usb.0","port":"1.5","drive":"libvirt-8-storage","id":"ua-test1","removable":false}'
\
+-blockdev
'{"driver":"file","filename":"/tmp/img6","node-name":"libvirt-7-storage","read-only":true}'
\
+-device
'{"driver":"usb-storage","bus":"usb.0","port":"1.6","drive":"libvirt-7-storage","id":"ua-test2","removable":false}'
\
+-blockdev
'{"driver":"file","filename":"/tmp/img7","node-name":"libvirt-6-storage","read-only":false}'
\
+-device
'{"driver":"usb-storage","bus":"usb.0","port":"1.7","drive":"libvirt-6-storage","id":"ua-test3","removable":false,"serial":"testserial3"}'
\
+-blockdev
'{"driver":"file","filename":"/tmp/img8","node-name":"libvirt-5-storage","read-only":true}'
\
+-device
'{"driver":"usb-storage","bus":"usb.0","port":"1.8","drive":"libvirt-5-storage","id":"ua-test4","removable":false,"serial":"testserial4"}'
\
+-blockdev
'{"driver":"file","filename":"/tmp/img9","node-name":"libvirt-4-storage","read-only":false}'
\
+-device
'{"driver":"usb-storage","bus":"usb.0","port":"2.1","drive":"libvirt-4-storage","id":"usb-disk8","removable":true}'
\
+-blockdev
'{"driver":"file","filename":"/tmp/imga","node-name":"libvirt-3-storage","read-only":false}'
\
+-device
'{"driver":"usb-storage","bus":"usb.0","port":"2.2","drive":"libvirt-3-storage","id":"usb-disk9","removable":true,"serial":"testserial5"}'
\
+-blockdev
'{"driver":"file","filename":"/tmp/imgb","node-name":"libvirt-2-storage","read-only":false}'
\
+-device
'{"driver":"usb-storage","bus":"usb.0","port":"2.3","drive":"libvirt-2-storage","id":"ua-test5","removable":true}'
\
+-blockdev
'{"driver":"file","filename":"/tmp/imgc","node-name":"libvirt-1-storage","read-only":false}'
\
+-device
'{"driver":"usb-storage","bus":"usb.0","port":"2.4","drive":"libvirt-1-storage","id":"ua-test6","removable":true,"serial":"testserial6"}'
\
-audiodev '{"id":"audio1","driver":"none"}' \
-device
'{"driver":"virtio-balloon-pci","id":"balloon0","bus":"pci.0","addr":"0x2"}' \
-sandbox
on,obsolete=deny,elevateprivileges=deny,spawn=deny,resourcecontrol=deny \
diff --git a/tests/qemuxmlconfdata/disk-usb-device.x86_64-latest.xml
b/tests/qemuxmlconfdata
[PATCH v2 06/13] qemu_capabilities: Introduce QEMU_CAPS_DEVICE_USB_BOT
From: Akihiko Odaki
usb-bot is supported by all supported QEMU versions; it is present since
1.4.0 and libvirt supports 4.2.0 or later.
Add a capability just in case USB_STORAGE_BOT is disabled when building
QEMU.
Signed-off-by: Akihiko Odaki
Signed-off-by: Peter Krempa
---
src/qemu/qemu_capabilities.c | 4
src/qemu/qemu_capabilities.h | 3 +++
tests/qemucapabilitiesdata/caps_10.0.0_aarch64.xml | 1 +
tests/qemucapabilitiesdata/caps_10.0.0_ppc64.xml | 1 +
tests/qemucapabilitiesdata/caps_10.0.0_s390x.xml | 1 +
tests/qemucapabilitiesdata/caps_10.0.0_x86_64+amdsev.xml | 1 +
tests/qemucapabilitiesdata/caps_10.0.0_x86_64.xml| 1 +
tests/qemucapabilitiesdata/caps_6.2.0_ppc64.xml | 1 +
tests/qemucapabilitiesdata/caps_6.2.0_x86_64.xml | 1 +
tests/qemucapabilitiesdata/caps_7.0.0_ppc64.xml | 1 +
tests/qemucapabilitiesdata/caps_7.0.0_x86_64.xml | 1 +
tests/qemucapabilitiesdata/caps_7.1.0_ppc64.xml | 1 +
tests/qemucapabilitiesdata/caps_7.1.0_x86_64.xml | 1 +
tests/qemucapabilitiesdata/caps_7.2.0_ppc.xml| 1 +
tests/qemucapabilitiesdata/caps_7.2.0_x86_64+hvf.xml | 1 +
tests/qemucapabilitiesdata/caps_7.2.0_x86_64.xml | 1 +
tests/qemucapabilitiesdata/caps_8.0.0_x86_64.xml | 1 +
tests/qemucapabilitiesdata/caps_8.1.0_s390x.xml | 1 +
tests/qemucapabilitiesdata/caps_8.1.0_x86_64.xml | 1 +
tests/qemucapabilitiesdata/caps_8.2.0_aarch64.xml| 1 +
tests/qemucapabilitiesdata/caps_8.2.0_armv7l.xml | 1 +
tests/qemucapabilitiesdata/caps_8.2.0_loongarch64.xml| 1 +
tests/qemucapabilitiesdata/caps_8.2.0_s390x.xml | 1 +
tests/qemucapabilitiesdata/caps_8.2.0_x86_64.xml | 1 +
tests/qemucapabilitiesdata/caps_9.0.0_x86_64.xml | 1 +
tests/qemucapabilitiesdata/caps_9.1.0_riscv64.xml| 1 +
tests/qemucapabilitiesdata/caps_9.1.0_s390x.xml | 1 +
tests/qemucapabilitiesdata/caps_9.1.0_x86_64.xml | 1 +
tests/qemucapabilitiesdata/caps_9.2.0_aarch64+hvf.xml| 1 +
tests/qemucapabilitiesdata/caps_9.2.0_s390x.xml | 1 +
tests/qemucapabilitiesdata/caps_9.2.0_x86_64+amdsev.xml | 1 +
tests/qemucapabilitiesdata/caps_9.2.0_x86_64.xml | 1 +
32 files changed, 37 insertions(+)
diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c
index 2ba5462bb2..f480060e50 100644
--- a/src/qemu/qemu_capabilities.c
+++ b/src/qemu/qemu_capabilities.c
@@ -736,6 +736,9 @@ VIR_ENUM_IMPL(virQEMUCaps,
"bus-floppy", /* QEMU_CAPS_BUS_FLOPPY */
"nvme", /* QEMU_CAPS_DEVICE_NVME */
"nvme-ns", /* QEMU_CAPS_DEVICE_NVME_NS */
+
+ /* 480 */
+ "usb-bot", /* QEMU_CAPS_DEVICE_USB_BOT */
);
@@ -1422,6 +1425,7 @@ struct virQEMUCapsStringFlags virQEMUCapsObjectTypes[] = {
{ "virtio-mem-ccw", QEMU_CAPS_DEVICE_VIRTIO_MEM_CCW },
{ "nvme", QEMU_CAPS_DEVICE_NVME },
{ "nvme-ns", QEMU_CAPS_DEVICE_NVME_NS },
+{ "usb-bot", QEMU_CAPS_DEVICE_USB_BOT },
};
diff --git a/src/qemu/qemu_capabilities.h b/src/qemu/qemu_capabilities.h
index 3c3d12159f..cef36bf64b 100644
--- a/src/qemu/qemu_capabilities.h
+++ b/src/qemu/qemu_capabilities.h
@@ -718,6 +718,9 @@ typedef enum { /* virQEMUCapsFlags grouping marker for
syntax-check */
QEMU_CAPS_DEVICE_NVME, /* -device nvme */
QEMU_CAPS_DEVICE_NVME_NS, /* -device nvme-ns */
+/* 480 */
+QEMU_CAPS_DEVICE_USB_BOT, /* -device usb-bot */
+
QEMU_CAPS_LAST /* this must always be the last item */
} virQEMUCapsFlags;
diff --git a/tests/qemucapabilitiesdata/caps_10.0.0_aarch64.xml
b/tests/qemucapabilitiesdata/caps_10.0.0_aarch64.xml
index 95752eb223..200873b3a2 100644
--- a/tests/qemucapabilitiesdata/caps_10.0.0_aarch64.xml
+++ b/tests/qemucapabilitiesdata/caps_10.0.0_aarch64.xml
@@ -163,6 +163,7 @@
+
1000
61700285
v10.0.0
diff --git a/tests/qemucapabilitiesdata/caps_10.0.0_ppc64.xml
b/tests/qemucapabilitiesdata/caps_10.0.0_ppc64.xml
index 12afb64af8..0c57798255 100644
--- a/tests/qemucapabilitiesdata/caps_10.0.0_ppc64.xml
+++ b/tests/qemucapabilitiesdata/caps_10.0.0_ppc64.xml
@@ -171,6 +171,7 @@
+
1000
42900285
v10.0.0
diff --git a/tests/qemucapabilitiesdata/caps_10.0.0_s390x.xml
b/tests/qemucapabilitiesdata/caps_10.0.0_s390x.xml
index 032b527188..070e673d0b 100644
--- a/tests/qemucapabilitiesdata/caps_10.0.0_s390x.xml
+++ b/tests/qemucapabilitiesdata/caps_10.0.0_s390x.xml
@@ -137,6 +137,7 @@
+
1000
39100285
v10.0.0
diff --git a/tests/qemucapabilitiesdata/caps_10.0.0_x86_64+amdsev.xml
b/tests/qemucapabilitiesdata/caps_10.0.0_x86_64+amdsev.xml
index ec79c6d4b1..bbfa007acc 100644
--- a/tests/qemucapabilitiesdata/caps_10.0.0_x86_64+amdsev.xml
+++ b/tests/qemucapabilitiesdata/caps_10.0.0_x86_64+amdsev.xml
@@ -208,6 +208,7 @@
+
1000
[PATCH v2 12/13] qemuxmlconftest: Prepare for proper testing in 'disk-cdrom-usb-empty'
From: Peter Krempa
Modify the validation of empty cdroms to trigger only for
VIR_DOMAIN_DISK_MODEL_USB_STORAGE as with 'usb-bot' we can properly
emulate a cdrom.
Signed-off-by: Peter Krempa
---
src/qemu/qemu_validate.c | 3 +-
...om-usb-empty.x86_64-latest.abi-update.args | 33
...rom-usb-empty.x86_64-latest.abi-update.xml | 38 +++
tests/qemuxmlconftest.c | 1 +
4 files changed, 74 insertions(+), 1 deletion(-)
create mode 100644
tests/qemuxmlconfdata/disk-cdrom-usb-empty.x86_64-latest.abi-update.args
create mode 100644
tests/qemuxmlconfdata/disk-cdrom-usb-empty.x86_64-latest.abi-update.xml
diff --git a/src/qemu/qemu_validate.c b/src/qemu/qemu_validate.c
index b2faf43002..7e0e4db516 100644
--- a/src/qemu/qemu_validate.c
+++ b/src/qemu/qemu_validate.c
@@ -3189,7 +3189,8 @@ qemuValidateDomainDeviceDefDiskFrontend(const
virDomainDiskDef *disk,
return -1;
}
-if (virStorageSourceIsEmpty(disk->src)) {
+if (disk->model == VIR_DOMAIN_DISK_MODEL_USB_STORAGE &&
+virStorageSourceIsEmpty(disk->src)) {
virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
_("'usb' disk must not be empty"));
return -1;
diff --git
a/tests/qemuxmlconfdata/disk-cdrom-usb-empty.x86_64-latest.abi-update.args
b/tests/qemuxmlconfdata/disk-cdrom-usb-empty.x86_64-latest.abi-update.args
new file mode 100644
index 00..cebc6e66e8
--- /dev/null
+++ b/tests/qemuxmlconfdata/disk-cdrom-usb-empty.x86_64-latest.abi-update.args
@@ -0,0 +1,33 @@
+LC_ALL=C \
+PATH=/bin \
+HOME=/var/lib/libvirt/qemu/domain--1-QEMUGuest1 \
+USER=test \
+LOGNAME=test \
+XDG_DATA_HOME=/var/lib/libvirt/qemu/domain--1-QEMUGuest1/.local/share \
+XDG_CACHE_HOME=/var/lib/libvirt/qemu/domain--1-QEMUGuest1/.cache \
+XDG_CONFIG_HOME=/var/lib/libvirt/qemu/domain--1-QEMUGuest1/.config \
+/usr/bin/qemu-system-x86_64 \
+-name guest=QEMUGuest1,debug-threads=on \
+-S \
+-object
'{"qom-type":"secret","id":"masterKey0","format":"raw","file":"/var/lib/libvirt/qemu/domain--1-QEMUGuest1/master-key.aes"}'
\
+-machine pc,usb=off,dump-guest-core=off,memory-backend=pc.ram,acpi=off \
+-accel tcg \
+-cpu qemu64 \
+-m size=219136k \
+-object '{"qom-type":"memory-backend-ram","id":"pc.ram","size":224395264}' \
+-overcommit mem-lock=off \
+-smp 1,sockets=1,cores=1,threads=1 \
+-uuid c7a5fdbd-edaf-9455-926a-d65c16db1809 \
+-display none \
+-no-user-config \
+-nodefaults \
+-chardev socket,id=charmonitor,fd=1729,server=on,wait=off \
+-mon chardev=charmonitor,id=monitor,mode=control \
+-rtc base=utc \
+-no-shutdown \
+-boot strict=on \
+-device
'{"driver":"piix3-usb-uhci","id":"usb","bus":"pci.0","addr":"0x1.0x2"}' \
+-device
'{"driver":"usb-storage","bus":"usb.0","port":"1","id":"usb-disk1","removable":false}'
\
+-audiodev '{"id":"audio1","driver":"none"}' \
+-sandbox
on,obsolete=deny,elevateprivileges=deny,spawn=deny,resourcecontrol=deny \
+-msg timestamp=on
diff --git
a/tests/qemuxmlconfdata/disk-cdrom-usb-empty.x86_64-latest.abi-update.xml
b/tests/qemuxmlconfdata/disk-cdrom-usb-empty.x86_64-latest.abi-update.xml
new file mode 100644
index 00..32aa011f90
--- /dev/null
+++ b/tests/qemuxmlconfdata/disk-cdrom-usb-empty.x86_64-latest.abi-update.xml
@@ -0,0 +1,38 @@
+
+ QEMUGuest1
+ c7a5fdbd-edaf-9455-926a-d65c16db1809
+ 219100
+ 219100
+ 1
+
+hvm
+
+
+
+qemu64
+
+
+ destroy
+ restart
+ destroy
+
+/usr/bin/qemu-system-x86_64
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
diff --git a/tests/qemuxmlconftest.c b/tests/qemuxmlconftest.c
index 858dde6ae8..db6f68cbe6 100644
--- a/tests/qemuxmlconftest.c
+++ b/tests/qemuxmlconftest.c
@@ -1545,6 +1545,7 @@ mymain(void)
DO_TEST_CAPS_LATEST("disk-cdrom");
DO_TEST_CAPS_LATEST("disk-cdrom-empty-network-invalid");
+DO_TEST_CAPS_ARCH_LATEST_ABI_UPDATE("disk-cdrom-usb-empty", "x86_64");
DO_TEST_CAPS_LATEST_PARSE_ERROR("disk-cdrom-usb-empty");
DO_TEST_CAPS_LATEST("disk-cdrom-network");
DO_TEST_CAPS_LATEST("disk-cdrom-tray");
--
2.49.0
[PATCH v2 10/13] qemuBuildDeviceAddresDriveProps: Prepare for 'drive' address for usb-bot disks
From: Peter Krempa
While the 'usb-storage' based disks use the USB address directly, with
'usb-bot' the USB address is on the "controller" part of the device and
the 'scsi-hd/cd' device will use a 'drive' address from qemu's PoV.
Since we do not want to expose the 'usb-bot' as explicit controller
to preserve compatibility with existing configs we plan to upgrade
implement the formatter for 'drive' address when the "diskbus" property
is VIR_DOMAIN_DISK_BUS_USB.
Signed-off-by: Peter Krempa
---
src/qemu/qemu_command.c | 15 ++-
1 file changed, 14 insertions(+), 1 deletion(-)
diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
index 4de6016784..910242a389 100644
--- a/src/qemu/qemu_command.c
+++ b/src/qemu/qemu_command.c
@@ -545,8 +545,21 @@ qemuBuildDeviceAddresDriveProps(virJSONValue *props,
return -1;
break;
-case VIR_DOMAIN_DISK_BUS_VIRTIO:
case VIR_DOMAIN_DISK_BUS_USB:
+/* Device info with type VIR_DOMAIN_DEVICE_ADDRESS_TYPE_DRIVE and
+ * VIR_DOMAIN_DISK_BUS_USB diskbus is an internal representation
+ * for the device address for 'usb-bot'. */
+bus = g_strdup_printf("%s.0", info->alias);
+
+if (virJSONValueObjectAdd(&props,
+ "s:bus", bus,
+ "u:scsi-id", info->addr.drive.target,
+ "u:lun", info->addr.drive.unit,
+ NULL) < 0)
+return -1;
+break;
+
+case VIR_DOMAIN_DISK_BUS_VIRTIO:
case VIR_DOMAIN_DISK_BUS_XEN:
case VIR_DOMAIN_DISK_BUS_UML:
case VIR_DOMAIN_DISK_BUS_SD:
--
2.49.0
[PATCH v2] util: workaround libxml2 lack of thread safe initialization
From: Daniel P. Berrangé
The main XML parser code global initializer historically had a mutex
protecting it, and more recently uses a pthread_once. The RelaxNG
code, however, relies on two other global initializers that are
not thread safe, just relying on setting an integer "initialized"
flag.
Calling the relevant initializers from libvirt in a protected global
initializer will protect libvirt's own concurrent usage, however, it
cannot protect against other libraries loaded in process that might
be using libxml2's schema code. Fortunately:
* The chances of other loaded non-libvirt code using libxml is
relatively low
* The chances of other loaded non-libvirt code using the schema
validation / catalog functionality inside libxml is even
lower
* The chances of both libvirt and the non-libvirt usage having
their *1st* usage of libxml2 be concurrent is tiny
IOW, in practice, although our solution doesn't fully fix the thread
safety, it is good enough.
libxml2 should none the less still be fixed to make its global
initializers be thread safe without special actions by its API
consumers[1].
Resolves: https://gitlab.com/libvirt/libvirt/-/issues/788
[1] https://gitlab.gnome.org/GNOME/libxml2/-/merge_requests/326
Signed-off-by: Daniel P. Berrangé
---
Changed in v3:
- Drop xmlInitializeCatalog - I misread the code wrt
thread safety - it has a sufficiently protective mutex
- Cope with return type change for xmlSchemaInitTypes
in libxml >= 2.11.0
src/util/virxml.c | 28
1 file changed, 28 insertions(+)
diff --git a/src/util/virxml.c b/src/util/virxml.c
index 9d46e5f32f..c851d6f407 100644
--- a/src/util/virxml.c
+++ b/src/util/virxml.c
@@ -26,6 +26,8 @@
#include
#include
+#include
+#include
#include "virerror.h"
#include "virxml.h"
@@ -35,6 +37,7 @@
#include "virstring.h"
#include "virutil.h"
#include "viruuid.h"
+#include "virthread.h"
#include "configmake.h"
#define VIR_FROM_THIS VIR_FROM_XML
@@ -50,6 +53,28 @@ struct virParserData {
};
+static int
+virXMLSchemaOnceInit(void)
+{
+#if LIBXML_VERSION >= 21100
+if (xmlSchemaInitTypes() < 0) {
+virReportError(VIR_ERR_INTERNAL_ERROR,
+ _("Unable to initialize libxml2 schema types"));
+return -1;
+}
+#else
+xmlSchemaInitTypes();
+#endif
+if (xmlRelaxNGInitTypes() < 0) {
+virReportError(VIR_ERR_INTERNAL_ERROR,
+ _("Unable to initialize libxml2 RelaxNG data"));
+return -1;
+}
+return 0;
+}
+
+VIR_ONCE_GLOBAL_INIT(virXMLSchema);
+
static xmlXPathContextPtr
virXMLXPathContextNew(xmlDocPtr xml)
{
@@ -1603,6 +1628,9 @@ virXMLValidatorInit(const char *schemafile)
{
g_autoptr(virXMLValidator) validator = NULL;
+if (virXMLSchemaInitialize() < 0)
+return NULL;
+
validator = g_new0(virXMLValidator, 1);
validator->schemafile = g_strdup(schemafile);
--
2.49.0
[PATCH v2 08/13] conf: introduce usb disk models 'usb-storage' and 'usb-bot'
From: Peter Krempa
Historically libvirt specified 'usb-storage' as driver for USB disks.
This though combined with '-blockdev' doesn't properly configure the
device to look like CDROM for .
'usb-bot' acts like a controler and allows explicitly connecting a
-device to it.
In qemu the devices share implementation so they are effectively
identical and can be used interchangably. There is though a slight
difference in how the storage device itself (the SCSI bit) looks when
CDROM as they were not declared as cdrom before.
As this is effectively a bugfix we'll be fixing the behaviour for the
default configuration. The possibility to explicitly set the model is
added as a possibility for working around possible problems if they'd
appear.
Signed-off-by: Peter Krempa
---
docs/formatdomain.rst | 23 +--
src/conf/domain_conf.c| 2 +
src/conf/domain_conf.h| 3 +
src/conf/schemas/domaincommon.rng | 2 +
src/qemu/qemu_domain_address.c| 2 +
.../disk-usb-device-model.x86_64-latest.args | 46 +
.../disk-usb-device-model.x86_64-latest.xml | 64 +++
.../qemuxmlconfdata/disk-usb-device-model.xml | 46 +
tests/qemuxmlconftest.c | 1 +
9 files changed, 185 insertions(+), 4 deletions(-)
create mode 100644
tests/qemuxmlconfdata/disk-usb-device-model.x86_64-latest.args
create mode 100644
tests/qemuxmlconfdata/disk-usb-device-model.x86_64-latest.xml
create mode 100644 tests/qemuxmlconfdata/disk-usb-device-model.xml
diff --git a/docs/formatdomain.rst b/docs/formatdomain.rst
index ae054a52b3..e4ebf061c7 100644
--- a/docs/formatdomain.rst
+++ b/docs/formatdomain.rst
@@ -2862,10 +2862,25 @@ paravirtualized driver is specified via the ``disk``
element.
``model``
Indicates the emulated device model of the disk. Typically this is
- indicated solely by the ``bus`` property but for ``bus`` "virtio" the
- model can be specified further with "virtio", "virtio-transitional" or
- "virtio-non-transitional". See `virtio device models`_
- for more details. :since:`Since 5.2.0`
+ indicated solely by the ``bus`` property.
+
+ For ``bus`` "virtio" the model can be specified further with "virtio",
+ "virtio-transitional" or "virtio-non-transitional". See `virtio device
+ models`_ for more details. :since:`Since 5.2.0`
+
+ For ``bus`` "usb" the model can be specified further with ``usb-storage``
+ or ``usb-bot``. There is no difference between the two models for
+ is properly exposed as a cdrom device inside the
+ guest OS. Unfortunately this configuration is not ABI compatible and thus
+ it can't be interchanged.
+
+ The QEMU hypervisor driver will pick ``usb-bot`` for cold starts or
+ hotplug for cdrom devices to properly configure the devices. This is
+ not compatible for migration to older versions of libvirt and explicit
+ configuration needs to be used.
+ :since:`Since 11.5.0`; relevant only for ``QEMU`` hypervisor.
+
``rawio``
Indicates whether the disk needs rawio capability. Valid settings are
"yes" or "no" (default is "no"). If any one disk in a domain has
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index 1a6c8afb1d..2882a7746b 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -1399,6 +1399,8 @@ VIR_ENUM_IMPL(virDomainDiskModel,
"virtio",
"virtio-transitional",
"virtio-non-transitional",
+ "usb-storage",
+ "usb-bot",
);
VIR_ENUM_IMPL(virDomainDiskMirrorState,
diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h
index 3d380073cf..73e8a2fb99 100644
--- a/src/conf/domain_conf.h
+++ b/src/conf/domain_conf.h
@@ -439,6 +439,9 @@ typedef enum {
VIR_DOMAIN_DISK_MODEL_VIRTIO_TRANSITIONAL,
VIR_DOMAIN_DISK_MODEL_VIRTIO_NON_TRANSITIONAL,
+VIR_DOMAIN_DISK_MODEL_USB_STORAGE,
+VIR_DOMAIN_DISK_MODEL_USB_BOT,
+
VIR_DOMAIN_DISK_MODEL_LAST
} virDomainDiskModel;
diff --git a/src/conf/schemas/domaincommon.rng
b/src/conf/schemas/domaincommon.rng
index b1fe51f519..a80005562a 100644
--- a/src/conf/schemas/domaincommon.rng
+++ b/src/conf/schemas/domaincommon.rng
@@ -1766,6 +1766,8 @@
virtio
virtio-transitional
virtio-non-transitional
+usb-storage
+usb-bot
diff --git a/src/qemu/qemu_domain_address.c b/src/qemu/qemu_domain_address.c
index bb86cfa0c3..3a23f70d39 100644
--- a/src/qemu/qemu_domain_address.c
+++ b/src/qemu/qemu_domain_address.c
@@ -729,6 +729,8 @@ qemuDomainDeviceCalculatePCIConnectFlags(virDomainDeviceDef
*dev,
case VIR_DOMAIN_DISK_MODEL_DEFAULT:
return virtioFlags;
case VIR_DOMAIN_DISK_MODEL_LAST:
+case VIR_DOMAIN_DISK_MODEL_USB_STORAGE:
+case VI
[PATCH v2 13/13] qemu: Replace usb-storage with usb-bot
From: Akihiko Odaki
usb-storage is a compound device that automatically creates a USB mass
storage device and a SCSI device as its backend. Unfortunately it lacks
some configuration options that are usually present with a SCSI device,
and cannot represent CD-ROM in particular.
Replace usb-storage with usb-bot, which can be combined with a manually
created SCSI device. libvirt will configure the SCSI device in a way
identical with how QEMU does for usb-storage except that now it respects
a configuration option to represent CD-ROM.
Resolves: https://gitlab.com/libvirt/libvirt/-/issues/368
Signed-off-by: Akihiko Odaki
Signed-off-by: Peter Krempa
---
src/qemu/qemu_alias.c | 20 -
src/qemu/qemu_capabilities.c | 3 +-
src/qemu/qemu_command.c | 86 ---
src/qemu/qemu_command.h | 5 ++
src/qemu/qemu_hotplug.c | 18
src/qemu/qemu_validate.c | 38 ++--
tests/qemuhotplugtest.c | 4 +-
...om-usb-empty.x86_64-latest.abi-update.args | 3 +-
.../disk-usb-device-model.x86_64-latest.args | 6 +-
...k-usb-device.x86_64-latest.abi-update.args | 12 ++-
10 files changed, 167 insertions(+), 28 deletions(-)
diff --git a/src/qemu/qemu_alias.c b/src/qemu/qemu_alias.c
index 9d39ebd63d..67cbddd470 100644
--- a/src/qemu/qemu_alias.c
+++ b/src/qemu/qemu_alias.c
@@ -268,8 +268,24 @@ qemuAssignDeviceDiskAlias(virDomainDef *def,
break;
case VIR_DOMAIN_DISK_BUS_USB:
-diskPriv->qomName =
g_strdup_printf("/machine/peripheral/%s/%s.0/legacy[0]",
-disk->info.alias,
disk->info.alias);
+switch (disk->model) {
+case VIR_DOMAIN_DISK_MODEL_USB_STORAGE:
+diskPriv->qomName =
g_strdup_printf("/machine/peripheral/%s/%s.0/legacy[0]",
+disk->info.alias,
disk->info.alias);
+break;
+
+case VIR_DOMAIN_DISK_MODEL_USB_BOT:
+diskPriv->qomName =
g_strdup_printf("/machine/peripheral/%s-disk",
+disk->info.alias);
+break;
+
+case VIR_DOMAIN_DISK_MODEL_DEFAULT:
+case VIR_DOMAIN_DISK_MODEL_VIRTIO:
+case VIR_DOMAIN_DISK_MODEL_VIRTIO_TRANSITIONAL:
+case VIR_DOMAIN_DISK_MODEL_VIRTIO_NON_TRANSITIONAL:
+case VIR_DOMAIN_DISK_MODEL_LAST:
+break;
+}
break;
case VIR_DOMAIN_DISK_BUS_XEN:
diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c
index f480060e50..ef6ec661ff 100644
--- a/src/qemu/qemu_capabilities.c
+++ b/src/qemu/qemu_capabilities.c
@@ -6472,7 +6472,8 @@ virQEMUCapsFillDomainDeviceDiskCaps(virQEMUCaps *qemuCaps,
VIR_DOMAIN_DISK_BUS_VIRTIO,
/* VIR_DOMAIN_DISK_BUS_SD */);
-if (virQEMUCapsGet(qemuCaps, QEMU_CAPS_DEVICE_USB_STORAGE))
+if (virQEMUCapsGet(qemuCaps, QEMU_CAPS_DEVICE_USB_STORAGE) ||
+virQEMUCapsGet(qemuCaps, QEMU_CAPS_DEVICE_USB_BOT))
VIR_DOMAIN_CAPS_ENUM_SET(disk->bus, VIR_DOMAIN_DISK_BUS_USB);
if (virQEMUCapsGet(qemuCaps, QEMU_CAPS_ICH9_AHCI))
diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
index 910242a389..98fd6fee85 100644
--- a/src/qemu/qemu_command.c
+++ b/src/qemu/qemu_command.c
@@ -1626,6 +1626,33 @@ qemuBuildIothreadMappingProps(GSList *iothreads)
return g_steal_pointer(&ret);
}
+int
+qemuBuildDiskBusProps(const virDomainDef *def,
+ const virDomainDiskDef *disk,
+ virJSONValue **propsRet)
+{
+g_autoptr(virJSONValue) props = NULL;
+
+*propsRet = NULL;
+
+if (disk->bus != VIR_DOMAIN_DISK_BUS_USB ||
+disk->model != VIR_DOMAIN_DISK_MODEL_USB_BOT)
+return 0;
+
+if (virJSONValueObjectAdd(&props,
+ "s:driver", "usb-bot",
+ "s:id", disk->info.alias,
+ "S:serial", disk->serial,
+ NULL) < 0)
+return -1;
+
+if (qemuBuildDeviceAddressProps(props, def, &disk->info) < 0)
+return -1;
+
+*propsRet = g_steal_pointer(&props);
+
+return 0;
+}
virJSONValue *
qemuBuildDiskDeviceProps(const virDomainDef *def,
@@ -1652,6 +1679,18 @@ qemuBuildDiskDeviceProps(const virDomainDef *def,
const char *rpolicy = NULL;
const char *model = NULL;
const char *product = NULL;
+const char *alias = disk->info.alias;
+g_autofree char *usbdiskalias = NULL;
+const virDomainDeviceInfo *deviceinfo = &disk->info;
+virDomainDeviceInfo usbSCSIinfo = {
+.type = VIR_DOMAIN_DEVICE_ADDRESS_TYPE_DRIVE,
+.addr.drive = { .diskbus = VIR_DOMAIN_DISK_BUS_USB },
+.effectiveBootIndex = deviceinfo->effectiveBootIndex,
[PATCH v2 04/13] qemuxmlconftest: Drop 'disk-cdrom-bus-other'
From: Peter Krempa
The test case is now redundant since 'disk-usb-device' case also tests
all cdrom configurations. Remove it.
Signed-off-by: Peter Krempa
---
.../disk-cdrom-bus-other.x86_64-latest.args | 34 -
.../disk-cdrom-bus-other.x86_64-latest.xml| 38 ---
.../qemuxmlconfdata/disk-cdrom-bus-other.xml | 30 ---
tests/qemuxmlconftest.c | 1 -
4 files changed, 103 deletions(-)
delete mode 100644
tests/qemuxmlconfdata/disk-cdrom-bus-other.x86_64-latest.args
delete mode 100644 tests/qemuxmlconfdata/disk-cdrom-bus-other.x86_64-latest.xml
delete mode 100644 tests/qemuxmlconfdata/disk-cdrom-bus-other.xml
diff --git a/tests/qemuxmlconfdata/disk-cdrom-bus-other.x86_64-latest.args
b/tests/qemuxmlconfdata/disk-cdrom-bus-other.x86_64-latest.args
deleted file mode 100644
index e1406af663..00
--- a/tests/qemuxmlconfdata/disk-cdrom-bus-other.x86_64-latest.args
+++ /dev/null
@@ -1,34 +0,0 @@
-LC_ALL=C \
-PATH=/bin \
-HOME=/var/lib/libvirt/qemu/domain--1-QEMUGuest1 \
-USER=test \
-LOGNAME=test \
-XDG_DATA_HOME=/var/lib/libvirt/qemu/domain--1-QEMUGuest1/.local/share \
-XDG_CACHE_HOME=/var/lib/libvirt/qemu/domain--1-QEMUGuest1/.cache \
-XDG_CONFIG_HOME=/var/lib/libvirt/qemu/domain--1-QEMUGuest1/.config \
-/usr/bin/qemu-system-x86_64 \
--name guest=QEMUGuest1,debug-threads=on \
--S \
--object
'{"qom-type":"secret","id":"masterKey0","format":"raw","file":"/var/lib/libvirt/qemu/domain--1-QEMUGuest1/master-key.aes"}'
\
--machine pc,usb=off,dump-guest-core=off,memory-backend=pc.ram,acpi=off \
--accel tcg \
--cpu qemu64 \
--m size=219136k \
--object '{"qom-type":"memory-backend-ram","id":"pc.ram","size":224395264}' \
--overcommit mem-lock=off \
--smp 1,sockets=1,cores=1,threads=1 \
--uuid c7a5fdbd-edaf-9455-926a-d65c16db1809 \
--display none \
--no-user-config \
--nodefaults \
--chardev socket,id=charmonitor,fd=1729,server=on,wait=off \
--mon chardev=charmonitor,id=monitor,mode=control \
--rtc base=utc \
--no-shutdown \
--boot strict=on \
--device
'{"driver":"piix3-usb-uhci","id":"usb","bus":"pci.0","addr":"0x1.0x2"}' \
--blockdev
'{"driver":"file","filename":"/root/boot.iso","node-name":"libvirt-1-storage","read-only":true}'
\
--device
'{"driver":"usb-storage","bus":"usb.0","port":"1","drive":"libvirt-1-storage","id":"usb-disk0","removable":false}'
\
--audiodev '{"id":"audio1","driver":"none"}' \
--sandbox
on,obsolete=deny,elevateprivileges=deny,spawn=deny,resourcecontrol=deny \
--msg timestamp=on
diff --git a/tests/qemuxmlconfdata/disk-cdrom-bus-other.x86_64-latest.xml
b/tests/qemuxmlconfdata/disk-cdrom-bus-other.x86_64-latest.xml
deleted file mode 100644
index f16fe63057..00
--- a/tests/qemuxmlconfdata/disk-cdrom-bus-other.x86_64-latest.xml
+++ /dev/null
@@ -1,38 +0,0 @@
-
- QEMUGuest1
- c7a5fdbd-edaf-9455-926a-d65c16db1809
- 219100
- 219100
- 1
-
-hvm
-
-
-
-qemu64
-
-
- destroy
- restart
- destroy
-
-/usr/bin/qemu-system-x86_64
-
-
-
-
-
-
-
-
-
-
-
-
-
-
-
-
-
-
-
diff --git a/tests/qemuxmlconfdata/disk-cdrom-bus-other.xml
b/tests/qemuxmlconfdata/disk-cdrom-bus-other.xml
deleted file mode 100644
index 119ce297a0..00
--- a/tests/qemuxmlconfdata/disk-cdrom-bus-other.xml
+++ /dev/null
@@ -1,30 +0,0 @@
-
- QEMUGuest1
- c7a5fdbd-edaf-9455-926a-d65c16db1809
- 219100
- 219100
- 1
-
-hvm
-
-
-
- destroy
- restart
- destroy
-
-/usr/bin/qemu-system-x86_64
-
-
-
-
-
-
-
-
-
-
-
-
-
-
diff --git a/tests/qemuxmlconftest.c b/tests/qemuxmlconftest.c
index 3f99026a31..dc93e59d34 100644
--- a/tests/qemuxmlconftest.c
+++ b/tests/qemuxmlconftest.c
@@ -1545,7 +1545,6 @@ mymain(void)
DO_TEST_CAPS_LATEST("disk-cdrom");
DO_TEST_CAPS_LATEST("disk-cdrom-empty-network-invalid");
-DO_TEST_CAPS_LATEST("disk-cdrom-bus-other");
DO_TEST_CAPS_LATEST_PARSE_ERROR("disk-cdrom-usb-empty");
DO_TEST_CAPS_LATEST("disk-cdrom-network");
DO_TEST_CAPS_LATEST("disk-cdrom-tray");
--
2.49.0
Re: virsh migrate fails when --copy-storage-all option is given!
On Mon, Jun 23, 2025 at 23:25:28 +0530, Anushree Mathur wrote:
> CC: libvirt devel list
>
> Hi Kevin/Peter,
>
> Thank you so much for addressing this issue. I tried out few more things and
> here is my analysis:
>
> Even when I removed the readonly option from guest xml I was still seeing the
> issue in migration,In the qemu-commandline I could still see auto-read-only
> option being set as true by default.
'auto-read-only' is very likely not the problem here. Due to the name
it's often suspected but it's really a red herring mostly.
What it does is that it instructs qemu just to automatically switch
between read-only and read-write as it did in the pre-blockdev era.
> Then I tried giving the auto-read-only as false in the guest xml with
> qemu-commandline param, it was actually getting set to false and the migration
> worked!
Okay, based on what you've wrote I'm now even more confused what you
wanted to do.
Based on previous report [1] you wanted to migrate with non-shared
storage (presence of --copy-storage-all which copies all read-write
disks to destination).
But now ...
> Steps I tried:
>
> 1) Started the guest with adding the snippet in the guest xml with parameters
> as:
>
>
>
> value='driver=file,filename=/disk_nfs/nfs/migrate_root.qcow2,node-name=drivefile,auto-read-only=false'/>
... this looks like shared storage ...
>
>
>
> value='virtio-blk-pci,drive=drive0,id=virtio-disk0,bus=pci.0,addr=0x5'/>
>
>
> 2) Started the migration and it worked.
... and especially since you added it via the qemu:arg backdoor, which
libvirt doesn't in any way interpret, it'd mean that --copy-storage-all
fully ignores what was declared here.
Thus it's weird that this would actually help anything especially when
you originally used option meant for non-shared storage.
So, how did you migrate this? What did you want to achieve?
> Could anyone please clarify from libvirt side what is the change required?
You need to first clarify what you are actually doing. This seems to be
different from what you tried last time.
For any further investigation I'll need:
- the full XML of the VM
- debug logs from the source [2]
- debug logs from the destination
- description what you are trying to achieve
>
> Thanks,
> Anushree-Mathur
[2] https://www.libvirt.org/kbase/debuglogs.html
>
>
> On 04/06/25 6:57 PM, Peter Krempa wrote:
> > On Wed, Jun 04, 2025 at 14:41:54 +0200, Kevin Wolf wrote:
> > > Am 28.05.2025 um 17:34 hat Peter Xu geschrieben:
> > > > Copy Kevin.
> > > >
> > > > On Wed, May 28, 2025 at 07:21:12PM +0530, Anushree Mathur wrote:
> > > > > Hi all,
> > > > >
> > > > >
> > > > > When I am trying to migrate the guest from host1 to host2 with the
> > > > > command
> > > > > line as follows:
> > > > >
> > > > > date;virsh migrate --live --domain guest1 qemu+ssh://dest/system
> > > > > --verbose
> > > > > --undefinesource --persistent --auto-converge --postcopy
> > > > > --copy-storage-all;date
> > > > >
> > > > > and it fails with the following error message-
> > > > >
> > > > > error: internal error: unable to execute QEMU command
> > > > > 'block-export-add':
> > > > > Block node is read-only
> > > > >
> > > > > HOST ENV:
> > > > >
> > > > > qemu : QEMU emulator version 9.2.2
> > > > > libvirt : libvirtd (libvirt) 11.1.0
> > > > > Seen with upstream qemu also
> > > > >
> > > > > Steps to reproduce:
> > > > > 1) Start the guest1
> > > > > 2) Migrate it with the command as
> > > > >
> > > > > date;virsh migrate --live --domain guest1 qemu+ssh://dest/system
> > > > > --verbose
> > > > > --undefinesource --persistent --auto-converge --postcopy
> > > > > --copy-storage-all;date
[1]
> > > > >
> > > > > 3) It fails as follows:
> > > > > error: internal error: unable to execute QEMU command
> > > > > 'block-export-add':
> > > > > Block node is read-only
> > > I assume this is about an inactive block node. Probably on the
> > > destination, but that's not clear to me from the error message.
> > Yes this would be on the destination. Libvirt exports the nodes on
> > destination, source connects and does the blockjob.
> >
> > The destination side is configured the same way as the source side so
> > if the source disk is configured as read-write the destination should be
> > as well
> >
> > > > > Things I analyzed-
> > > > > 1) This issue is not happening if I give --unsafe option in the virsh
> > > > > migrate command
> > This is weird; this shouldn't have any impact.
> >
> > > What does this translate to on the QEMU command line?
> > >
> > > > > 2) O/P of qemu-monitor command also shows ro as false
> > > > >
> > > > > virsh qemu-monitor-command guest1 --pretty --cmd '{ "execute":
> > > > > "query-block"
> > it'd be impossible to execute this on the guest due to timing; you'll
> > need to collect libvirt debug logs to do that:
> >
> > https://www.libvirt.org/kbase/debuglogs.html#tl-dr-enable-debug-logs-for-most-common-scenario
> >
> > I also thin
Re: [PATCH v2] util: workaround libxml2 lack of thread safe initialization
On Mon, Jun 23, 2025 at 17:17:06 +0100, Daniel P. Berrangé via Devel wrote: > From: Daniel P. Berrangé > > The main XML parser code global initializer historically had a mutex > protecting it, and more recently uses a pthread_once. The RelaxNG > code, however, relies on two other global initializers that are > not thread safe, just relying on setting an integer "initialized" > flag. > > Calling the relevant initializers from libvirt in a protected global > initializer will protect libvirt's own concurrent usage, however, it > cannot protect against other libraries loaded in process that might > be using libxml2's schema code. Fortunately: > > * The chances of other loaded non-libvirt code using libxml is >relatively low > * The chances of other loaded non-libvirt code using the schema >validation / catalog functionality inside libxml is even >lower > * The chances of both libvirt and the non-libvirt usage having >their *1st* usage of libxml2 be concurrent is tiny > > IOW, in practice, although our solution doesn't fully fix the thread > safety, it is good enough. > > libxml2 should none the less still be fixed to make its global > initializers be thread safe without special actions by its API > consumers[1]. > > Resolves: https://gitlab.com/libvirt/libvirt/-/issues/788 > [1] https://gitlab.gnome.org/GNOME/libxml2/-/merge_requests/326 > Signed-off-by: Daniel P. Berrangé > --- > > Changed in v3: > > - Drop xmlInitializeCatalog - I misread the code wrt >thread safety - it has a sufficiently protective mutex > - Cope with return type change for xmlSchemaInitTypes >in libxml >= 2.11.0 Reviewed-by: Peter Krempa
[PATCH v2 07/13] qemuxmlconftest: Invoke "disk-usb-device" case also without QEMU_CAPS_DEVICE_USB_BOT and with ABI_UPDATE
From: Peter Krempa
The QEMU_CAPS_DEVICE_USB_BOT device can be compiled out but
realistically it makes no sense to do it thus also makes no sense to
have another variant of input data for it.
Add another invocation of "disk-usb-device" clearing QEMU_CAPS_DEVICE_USB_BOT
to show the fallback code paths.
Also add "ABI_UPDATE" version for the two cases above as the ABI of
usb-bot cdrom is not migration-compatible and we'll be wanting to update
to the fixed configuration.
Signed-off-by: Peter Krempa
---
...est.QEMU_CAPS_DEVICE_USB_BOT-disabled.args | 59 +
...test.QEMU_CAPS_DEVICE_USB_BOT-disabled.xml | 107 +++
...ate.QEMU_CAPS_DEVICE_USB_BOT-disabled.args | 59 +
...date.QEMU_CAPS_DEVICE_USB_BOT-disabled.xml | 125 ++
...k-usb-device.x86_64-latest.abi-update.args | 59 +
...sk-usb-device.x86_64-latest.abi-update.xml | 125 ++
tests/qemuxmlconftest.c | 12 ++
7 files changed, 546 insertions(+)
create mode 100644
tests/qemuxmlconfdata/disk-usb-device.x86_64-latest.QEMU_CAPS_DEVICE_USB_BOT-disabled.args
create mode 100644
tests/qemuxmlconfdata/disk-usb-device.x86_64-latest.QEMU_CAPS_DEVICE_USB_BOT-disabled.xml
create mode 100644
tests/qemuxmlconfdata/disk-usb-device.x86_64-latest.abi-update.QEMU_CAPS_DEVICE_USB_BOT-disabled.args
create mode 100644
tests/qemuxmlconfdata/disk-usb-device.x86_64-latest.abi-update.QEMU_CAPS_DEVICE_USB_BOT-disabled.xml
create mode 100644
tests/qemuxmlconfdata/disk-usb-device.x86_64-latest.abi-update.args
create mode 100644
tests/qemuxmlconfdata/disk-usb-device.x86_64-latest.abi-update.xml
diff --git
a/tests/qemuxmlconfdata/disk-usb-device.x86_64-latest.QEMU_CAPS_DEVICE_USB_BOT-disabled.args
b/tests/qemuxmlconfdata/disk-usb-device.x86_64-latest.QEMU_CAPS_DEVICE_USB_BOT-disabled.args
new file mode 100644
index 00..079dfe5d99
--- /dev/null
+++
b/tests/qemuxmlconfdata/disk-usb-device.x86_64-latest.QEMU_CAPS_DEVICE_USB_BOT-disabled.args
@@ -0,0 +1,59 @@
+LC_ALL=C \
+PATH=/bin \
+HOME=/var/lib/libvirt/qemu/domain--1-QEMUGuest1 \
+USER=test \
+LOGNAME=test \
+XDG_DATA_HOME=/var/lib/libvirt/qemu/domain--1-QEMUGuest1/.local/share \
+XDG_CACHE_HOME=/var/lib/libvirt/qemu/domain--1-QEMUGuest1/.cache \
+XDG_CONFIG_HOME=/var/lib/libvirt/qemu/domain--1-QEMUGuest1/.config \
+/usr/bin/qemu-system-x86_64 \
+-name guest=QEMUGuest1,debug-threads=on \
+-S \
+-object
'{"qom-type":"secret","id":"masterKey0","format":"raw","file":"/var/lib/libvirt/qemu/domain--1-QEMUGuest1/master-key.aes"}'
\
+-machine pc,usb=off,dump-guest-core=off,memory-backend=pc.ram,acpi=off \
+-accel tcg \
+-cpu qemu64 \
+-m size=219136k \
+-object '{"qom-type":"memory-backend-ram","id":"pc.ram","size":224395264}' \
+-overcommit mem-lock=off \
+-smp 1,sockets=1,cores=1,threads=1 \
+-uuid c7a5fdbd-edaf-9455-926a-d65c16db1809 \
+-display none \
+-no-user-config \
+-nodefaults \
+-chardev socket,id=charmonitor,fd=1729,server=on,wait=off \
+-mon chardev=charmonitor,id=monitor,mode=control \
+-rtc base=utc \
+-no-shutdown \
+-boot strict=on \
+-device
'{"driver":"piix3-usb-uhci","id":"usb","bus":"pci.0","addr":"0x1.0x2"}' \
+-device '{"driver":"usb-hub","id":"hub0","bus":"usb.0","port":"1"}' \
+-device '{"driver":"usb-hub","id":"hub1","bus":"usb.0","port":"2"}' \
+-blockdev
'{"driver":"file","filename":"/tmp/img1","node-name":"libvirt-12-storage","read-only":false}'
\
+-device
'{"driver":"usb-storage","bus":"usb.0","port":"1.1","drive":"libvirt-12-storage","id":"usb-disk0","bootindex":1,"removable":false}'
\
+-blockdev
'{"driver":"file","filename":"/tmp/img2","node-name":"libvirt-11-storage","read-only":true}'
\
+-device
'{"driver":"usb-storage","bus":"usb.0","port":"1.2","drive":"libvirt-11-storage","id":"usb-disk1","removable":false}'
\
+-blockdev
'{"driver":"file","filename":"/tmp/img3","node-name":"libvirt-10-storage","read-only":false}'
\
+-device
'{"driver":"usb-storage","bus":"usb.0","port":"1.3","drive":"libvirt-10-storage","id":"usb-disk2","removable":false,"serial":"testserial1"}'
\
+-blockdev
'{"driver":"file","filename":"/tmp/img4","node-name":"libvirt-9-storage","read-only":true}'
\
+-device
'{"driver":"usb-storage","bus":"usb.0","port":"1.4","drive":"libvirt-9-storage","id":"usb-disk3","removable":false,"serial":"testserial2"}'
\
+-blockdev
'{"driver":"file","filename":"/tmp/img5","node-name":"libvirt-8-storage","read-only":false}'
\
+-device
'{"driver":"usb-storage","bus":"usb.0","port":"1.5","drive":"libvirt-8-storage","id":"ua-test1","removable":false}'
\
+-blockdev
'{"driver":"file","filename":"/tmp/img6","node-name":"libvirt-7-storage","read-only":true}'
\
+-device
'{"driver":"usb-storage","bus":"usb.0","port":"1.6","drive":"libvirt-7-storage","id":"ua-test2","removable":false}'
\
+-blockdev
'{"driver":"file","filename":"/tmp/img7","node-name":"libvirt-6-storage","read-only":false}'
\
+-device
'{"driver":"usb-storage","bus":"usb.0","port":"1.7","drive":"libvirt-6-storag
[PATCH v2 11/13] qemu: monitor: Introduce 'qemuMonitorSetUSBDiskAttached'
From: Peter Krempa
The helper sets the 'attached' property of the 'usb-bot' device to true,
which will be used on the hotplug code path.
Signed-off-by: Peter Krempa
---
src/qemu/qemu_monitor.c | 12
src/qemu/qemu_monitor.h | 3 +++
src/qemu/qemu_monitor_json.c | 20
src/qemu/qemu_monitor_json.h | 5 +
4 files changed, 40 insertions(+)
diff --git a/src/qemu/qemu_monitor.c b/src/qemu/qemu_monitor.c
index d508f50ed6..95c88fd5e8 100644
--- a/src/qemu/qemu_monitor.c
+++ b/src/qemu/qemu_monitor.c
@@ -2164,6 +2164,18 @@ qemuMonitorSetDBusVMStateIdList(qemuMonitor *mon,
}
+int
+qemuMonitorSetUSBDiskAttached(qemuMonitor *mon,
+ const char *alias)
+{
+QEMU_CHECK_MONITOR(mon);
+
+VIR_DEBUG("alias=%s", alias);
+
+return qemuMonitorJSONSetUSBDiskAttached(mon, alias);
+}
+
+
/**
* qemuMonitorGetMigrationParams:
* @mon: Pointer to the monitor object.
diff --git a/src/qemu/qemu_monitor.h b/src/qemu/qemu_monitor.h
index 51b65b4019..6030c31598 100644
--- a/src/qemu/qemu_monitor.h
+++ b/src/qemu/qemu_monitor.h
@@ -768,6 +768,9 @@ int qemuMonitorSavePhysicalMemory(qemuMonitor *mon,
int qemuMonitorSetDBusVMStateIdList(qemuMonitor *mon,
GSList *list);
+int qemuMonitorSetUSBDiskAttached(qemuMonitor *mon,
+ const char *alias);
+
int qemuMonitorGetMigrationParams(qemuMonitor *mon,
virJSONValue **params);
int qemuMonitorSetMigrationParams(qemuMonitor *mon,
diff --git a/src/qemu/qemu_monitor_json.c b/src/qemu/qemu_monitor_json.c
index a6fb2a2013..5297ffb027 100644
--- a/src/qemu/qemu_monitor_json.c
+++ b/src/qemu/qemu_monitor_json.c
@@ -2086,6 +2086,26 @@ qemuMonitorJSONSetDBusVMStateIdList(qemuMonitor *mon,
}
+/**
+ * qemuMonitorJSONSetUSBDiskAttached:
+ * @mon: monitor object
+ * @alias: alias of usb disk to set
+ *
+ * Sets the 'attached' property of @alias to true.
+ */
+int
+qemuMonitorJSONSetUSBDiskAttached(qemuMonitor *mon,
+ const char *alias)
+{
+qemuMonitorJSONObjectProperty prop = {
+.type = QEMU_MONITOR_OBJECT_PROPERTY_BOOLEAN,
+.val.b = true,
+};
+
+return qemuMonitorJSONSetObjectProperty(mon, alias, "attached", &prop);
+}
+
+
/* qemuMonitorJSONQueryNamedBlockNodes:
* @mon: Monitor pointer
*
diff --git a/src/qemu/qemu_monitor_json.h b/src/qemu/qemu_monitor_json.h
index 7f07e55e06..bd437f7938 100644
--- a/src/qemu/qemu_monitor_json.h
+++ b/src/qemu/qemu_monitor_json.h
@@ -757,6 +757,11 @@ qemuMonitorJSONSetDBusVMStateIdList(qemuMonitor *mon,
const char *idstr)
ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2) ATTRIBUTE_NONNULL(3);
+int
+qemuMonitorJSONSetUSBDiskAttached(qemuMonitor *mon,
+ const char *alias)
+ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2);
+
int
qemuMonitorJSONGetCPUMigratable(qemuMonitor *mon,
const char *cpuQOMPath,
--
2.49.0
[PATCH v2 03/13] qemusecuritytest: Use 'disk-usb-device' case instead of 'disk-cdrom-bus-other'
From: Peter Krempa
Signed-off-by: Peter Krempa
---
tests/qemusecuritytest.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/tests/qemusecuritytest.c b/tests/qemusecuritytest.c
index 88c8617d69..ba9506d0a0 100644
--- a/tests/qemusecuritytest.c
+++ b/tests/qemusecuritytest.c
@@ -207,7 +207,7 @@ mymain(void)
DO_TEST_DOMAIN("disk-backing-chains-noindex");
DO_TEST_DOMAIN("disk-cache");
DO_TEST_DOMAIN("disk-cdrom");
-DO_TEST_DOMAIN("disk-cdrom-bus-other");
+DO_TEST_DOMAIN("disk-usb-device");
DO_TEST_DOMAIN("disk-cdrom-network");
DO_TEST_DOMAIN("disk-cdrom-tray");
DO_TEST_DOMAIN("disk-copy_on_read");
--
2.49.0
[PATCH v2 01/13] qemuhotplugtest: Use VIR_DOMAIN_DEF_PARSE_ABI_UPDATE for virDomainDeviceDefParse
From: Peter Krempa The qemu hotplug code parses the device with ABI updates enabled so qemuhotplugtest ought to do the same. Signed-off-by: Peter Krempa --- tests/qemuhotplugtest.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tests/qemuhotplugtest.c b/tests/qemuhotplugtest.c index d2a1f5acf1..fdb5093549 100644 --- a/tests/qemuhotplugtest.c +++ b/tests/qemuhotplugtest.c @@ -152,7 +152,7 @@ testQemuHotplug(const void *data) const char *const *tmp; bool fail = test->fail; bool keep = test->keep; -unsigned int device_parse_flags = 0; +unsigned int device_parse_flags = VIR_DOMAIN_DEF_PARSE_ABI_UPDATE; virDomainObj *vm = NULL; g_autoptr(virDomainDeviceDef) dev = NULL; g_autoptr(qemuMonitorTest) test_mon = NULL; @@ -191,7 +191,7 @@ testQemuHotplug(const void *data) } if (test->action == ATTACH) -device_parse_flags = VIR_DOMAIN_DEF_PARSE_INACTIVE; +device_parse_flags |= VIR_DOMAIN_DEF_PARSE_INACTIVE; if (!(dev = virDomainDeviceDefParse(device_xml, vm->def, driver.xmlopt, NULL, -- 2.49.0
