- Devel - libvirt List Archives

[PATCH] conf: Check for bandwidth limits during parsing
by Michal Privoznik 13 Aug '24

13 Aug '24

The 'tc' program stores speeds in 64bit integers (unit is bytes per second) and sizes in uints (unit is bytes). We use different units: kilobytes per second and kibibytes and therefore we can parse values larger than 'tc' can handle. Reject those values right away. And while at it, fix the schema which assumed speed values fit into uint. Resolves: https://issues.redhat.com/browse/RHEL-45200 Signed-off-by: Michal Privoznik <mprivozn(a)redhat.com> --- src/conf/netdev_bandwidth_conf.c | 17 +++++++++++++++++ src/conf/schemas/networkcommon.rng | 2 +- 2 files changed, 18 insertions(+), 1 deletion(-) diff --git a/src/conf/netdev_bandwidth_conf.c b/src/conf/netdev_bandwidth_conf.c index 9faa46a27f..f3f0b2209a 100644 --- a/src/conf/netdev_bandwidth_conf.c +++ b/src/conf/netdev_bandwidth_conf.c @@ -24,6 +24,16 @@ #define VIR_FROM_THIS VIR_FROM_NONE +#define CHECK_LIMIT(val, limit, name) \ + do { \ + if ((val) > (limit)) { \ + virReportError(VIR_ERR_OVERFLOW, \ + _("value '%1$llu' is too big for '%2$s' parameter, maximum is '%3$llu'"), \ + val, name, (unsigned long long) limit); \ + return -1; \ + } \ + } while (0) + static int virNetDevBandwidthParseRate(xmlNodePtr node, virNetDevBandwidthRate *rate, @@ -50,6 +60,11 @@ virNetDevBandwidthParseRate(xmlNodePtr node, &rate->floor)) < 0) return -1; + CHECK_LIMIT(rate->average, 1ULL << 54, "average"); + CHECK_LIMIT(rate->peak, 1ULL << 54, "peak"); + CHECK_LIMIT(rate->burst, UINT_MAX >> 10, "burst"); + CHECK_LIMIT(rate->floor, 1ULL << 54, "floor"); + if (!rc_average && !rc_floor) { virReportError(VIR_ERR_XML_DETAIL, "%s", _("Missing mandatory average or floor attributes")); @@ -71,6 +86,8 @@ virNetDevBandwidthParseRate(xmlNodePtr node, return 0; } +#undef CHECK_LIMIT + /** * virNetDevBandwidthParse: * @bandwidth: parsed bandwidth diff --git a/src/conf/schemas/networkcommon.rng b/src/conf/schemas/networkcommon.rng index 6df6d43f54..2b3f902ffe 100644 --- a/src/conf/schemas/networkcommon.rng +++ b/src/conf/schemas/networkcommon.rng @@ -180,7 +180,7 @@ </define> <define name="speed"> - <data type="unsignedInt"> + <data type="unsignedLong"> <param name="pattern">[0-9]+</param> <param name="minInclusive">1</param> </data> -- 2.44.2

3 2

[PATCH] qemu: hotplug: Rollback FD passthrough for 'slirpfd' and 'vdpafd' on hotplug failure
by Peter Krempa 13 Aug '24

13 Aug '24

On failure to plug the device the cleanup path didn't roll back the FD passing to qemu thus qemu would hold the FDs indefinitely. Resolves: https://issues.redhat.com/browse/RHEL-53964 Fixes: b79abf9c3cdab8bcecfa8769629a4cdf4bf0b6c3 (vdpafd) Signed-off-by: Peter Krempa <pkrempa(a)redhat.com> --- src/qemu/qemu_hotplug.c | 3 +++ 1 file changed, 3 insertions(+) diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c index 7cb1800504..75b97cf736 100644 --- a/src/qemu/qemu_hotplug.c +++ b/src/qemu/qemu_hotplug.c @@ -1456,6 +1456,9 @@ qemuDomainAttachNetDevice(virQEMUDriver *driver, VIR_WARN("Failed to remove network backend for netdev %s", netdev_name); + qemuFDPassTransferMonitorRollback(netpriv->vdpafd, priv->mon); + qemuFDPassDirectTransferMonitorRollback(netpriv->slirpfd, priv->mon); + for (n = netpriv->tapfds; n; n = n->next) qemuFDPassDirectTransferMonitorRollback(n->data, priv->mon); -- 2.46.0

2 1

[PATCH] virnetlibsshsession: Reflect API change in libssh
by Michal Privoznik 12 Aug '24

12 Aug '24

As of libssh commit of libssh-0.11.0~70 [1] the ssh_channel_get_exit_status() function is deprecated and a new one is introduced instead: ssh_channel_get_exit_state(). It's not a drop-in replacement, but it's simple enough. Adapt our libssh handling code to this change. 1: https://git.libssh.org/projects/libssh.git/commit/?id=04d86aeeae73c78af8b3d… Signed-off-by: Michal Privoznik <mprivozn(a)redhat.com> --- meson.build | 3 +++ src/rpc/virnetlibsshsession.c | 39 +++++++++++++++++++++++++++++------ 2 files changed, 36 insertions(+), 6 deletions(-) diff --git a/meson.build b/meson.build index 06d88ad1f3..f31485c395 100644 --- a/meson.build +++ b/meson.build @@ -1096,6 +1096,9 @@ if conf.has('WITH_REMOTE') libssh_dep = dependency('libssh', version: '>=' + libssh_version, required: get_option('libssh')) if libssh_dep.found() conf.set('WITH_LIBSSH', 1) + if cc.has_function('ssh_channel_get_exit_state', dependencies: libssh_dep) + conf.set('WITH_SSH_CHANNEL_GET_EXIT_STATE', 1) + endif endif else libssh_dep = dependency('', required: false) diff --git a/src/rpc/virnetlibsshsession.c b/src/rpc/virnetlibsshsession.c index 6632e4a9ef..e496d3334e 100644 --- a/src/rpc/virnetlibsshsession.c +++ b/src/rpc/virnetlibsshsession.c @@ -170,6 +170,25 @@ virNetLibsshSessionOnceInit(void) } VIR_ONCE_GLOBAL_INIT(virNetLibsshSession); + +static int virNetLibsshChannelGetExitStatus(ssh_channel channel, + uint32_t *exit_status) +{ +#ifdef WITH_SSH_CHANNEL_GET_EXIT_STATE + return ssh_channel_get_exit_state(channel, exit_status, NULL, NULL); +#else + int rc; + + rc = *exit_status = ssh_channel_get_exit_status(channel); + + if (rc != SSH_OK) + return SSH_ERROR; + + return *exit_status; +#endif +} + + static virNetLibsshAuthMethod * virNetLibsshSessionAuthMethodNew(virNetLibsshSession *sess) { @@ -1179,12 +1198,16 @@ virNetLibsshChannelRead(virNetLibsshSession *sess, } if (ssh_channel_is_eof(sess->channel)) { + uint32_t exit_status; + int rc; eof: - if (ssh_channel_get_exit_status(sess->channel)) { + + rc = virNetLibsshChannelGetExitStatus(sess->channel, &exit_status); + if (rc != SSH_OK || exit_status != 0) { virReportError(VIR_ERR_LIBSSH, _("Remote command terminated with non-zero code: %1$d"), - ssh_channel_get_exit_status(sess->channel)); - sess->channelCommandReturnValue = ssh_channel_get_exit_status(sess->channel); + exit_status); + sess->channelCommandReturnValue = exit_status; sess->state = VIR_NET_LIBSSH_STATE_ERROR_REMOTE; virObjectUnlock(sess); return -1; @@ -1227,12 +1250,16 @@ virNetLibsshChannelWrite(virNetLibsshSession *sess, } if (ssh_channel_is_eof(sess->channel)) { - if (ssh_channel_get_exit_status(sess->channel)) { + uint32_t exit_status; + int rc; + + rc = virNetLibsshChannelGetExitStatus(sess->channel, &exit_status); + if (rc != SSH_OK || exit_status != 0) { virReportError(VIR_ERR_LIBSSH, _("Remote program terminated with non-zero code: %1$d"), - ssh_channel_get_exit_status(sess->channel)); + exit_status); sess->state = VIR_NET_LIBSSH_STATE_ERROR_REMOTE; - sess->channelCommandReturnValue = ssh_channel_get_exit_status(sess->channel); + sess->channelCommandReturnValue = exit_status; ret = -1; goto cleanup; -- 2.44.2

2 1

[PATCH] rpc: adapt for deprecation of ssh_channel_get_exit_status
by Daniel P. Berrangé 12 Aug '24

12 Aug '24

This method is deprecated in libssh 0.11.0 in favour of the more flexible ssh_channel_get_exit_state. Rewrite our code to call the latter, and provide a compat shim that only fills out exit status. Signed-off-by: Daniel P. Berrangé <berrange(a)redhat.com> --- src/rpc/virnetlibsshsession.c | 50 +++++++++++++++++++++++++++++------ 1 file changed, 42 insertions(+), 8 deletions(-) diff --git a/src/rpc/virnetlibsshsession.c b/src/rpc/virnetlibsshsession.c index 6632e4a9ef..faf7307725 100644 --- a/src/rpc/virnetlibsshsession.c +++ b/src/rpc/virnetlibsshsession.c @@ -48,6 +48,22 @@ VIR_LOG_INIT("rpc.netlibsshsession"); */ #define TRACE_LIBSSH 0 +#if LIBSSH_VERSION_INT < SSH_VERSION_INT(0, 11, 0) +static int ssh_channel_get_exit_state(ssh_channel channel, + uint32_t *pexit_code, + char **pexit_signal, + int *pcore_dumped) +{ + if (pexit_code) + *pexit_code = ssh_channel_get_exit_status(channel); + if (pexit_signal) + *pexit_signal = 0; + if (pcore_dumped) + *pcore_dumped = 0; + return 0; +} +#endif + typedef enum { VIR_NET_LIBSSH_STATE_NEW, VIR_NET_LIBSSH_STATE_HANDSHAKE_COMPLETE, @@ -1179,12 +1195,21 @@ virNetLibsshChannelRead(virNetLibsshSession *sess, } if (ssh_channel_is_eof(sess->channel)) { + uint32_t status; eof: - if (ssh_channel_get_exit_status(sess->channel)) { + if (ssh_channel_get_exit_state(sess->channel, &status, NULL, NULL) != 0) { + virReportError(VIR_ERR_LIBSSH, "%s", + _("Unable to fetch remote command exit status")); + sess->channelCommandReturnValue = 0; + sess->state = VIR_NET_LIBSSH_STATE_ERROR_REMOTE; + virObjectUnlock(sess); + return -1; + } + if (status != 0) { virReportError(VIR_ERR_LIBSSH, _("Remote command terminated with non-zero code: %1$d"), - ssh_channel_get_exit_status(sess->channel)); - sess->channelCommandReturnValue = ssh_channel_get_exit_status(sess->channel); + status); + sess->channelCommandReturnValue = status; sess->state = VIR_NET_LIBSSH_STATE_ERROR_REMOTE; virObjectUnlock(sess); return -1; @@ -1227,15 +1252,24 @@ virNetLibsshChannelWrite(virNetLibsshSession *sess, } if (ssh_channel_is_eof(sess->channel)) { - if (ssh_channel_get_exit_status(sess->channel)) { + uint32_t status; + if (ssh_channel_get_exit_state(sess->channel, &status, NULL, NULL) != 0) { + virReportError(VIR_ERR_LIBSSH, "%s", + _("Unable to fetch remote command exit status")); + sess->channelCommandReturnValue = 0; + sess->state = VIR_NET_LIBSSH_STATE_ERROR_REMOTE; + ret = -1; + goto cleanup; + } + if (status != 0) { virReportError(VIR_ERR_LIBSSH, - _("Remote program terminated with non-zero code: %1$d"), - ssh_channel_get_exit_status(sess->channel)); + _("Remote command terminated with non-zero code: %1$d"), + status); + sess->channelCommandReturnValue = status; sess->state = VIR_NET_LIBSSH_STATE_ERROR_REMOTE; - sess->channelCommandReturnValue = ssh_channel_get_exit_status(sess->channel); - ret = -1; goto cleanup; + } sess->state = VIR_NET_LIBSSH_STATE_CLOSED; -- 2.45.2

2 1

[PATCH] glibcompat: remove obsolete clang workaround
by Daniel P. Berrangé 12 Aug '24

12 Aug '24

This mostly reverts commit 65491a2dfe00bfcf9f09a8d6eab60234b56c8cc4. There was a bug introduced in glib 2.67.0 which impacted libvirt with clang causing -Wincompatible-pointer-types-discards-qualifiers warnings. This was actually fixed quite quickly in 2.67.1 with https://gitlab.gnome.org/GNOME/glib/-/merge_requests/1719 Our workaround was then broken with glib 2.81.1 due to commit 14b3d5da9019150d821f6178a075d85044b4c255 changing the signature of the (private) macro we were overriding. Since odd-number glib releases are development snapshots, and the original problem was only present in 2.67.0 and no other releases, just drop the workaround entirely. Signed-off-by: Daniel P. Berrangé <berrange(a)redhat.com> --- src/util/glibcompat.h | 28 +--------------------------- 1 file changed, 1 insertion(+), 27 deletions(-) diff --git a/src/util/glibcompat.h b/src/util/glibcompat.h index 3518023a41..474ff95bc5 100644 --- a/src/util/glibcompat.h +++ b/src/util/glibcompat.h @@ -22,33 +22,7 @@ #include <glib/gstdio.h> #include <glib-object.h> -#if GLIB_CHECK_VERSION(2, 67, 0) - -# if defined(__clang__) - -/* - * Clang detects (valid) issue in G_DEFINE_TYPE and derivatives starting with - * glib >= 2.67.0. See https://gitlab.gnome.org/GNOME/glib/-/issues/600 - * - * For that we need to disable the one check that produces an error in our - * builds when using any G_DEFINE_TYPE* macro. Thankfully all those macros end - * up using _G_DEFINE_TYPE_EXTENDED_BEGIN. Because with that we can redefine - * this one macro to cover all use cases. The macro is defined the same way it - * is defined in glib (with a very low probability of being changed thanks to a - * comment above it). - */ -# undef _G_DEFINE_TYPE_EXTENDED_BEGIN - -# define _G_DEFINE_TYPE_EXTENDED_BEGIN(TypeName, type_name, TYPE_PARENT, flags) \ - _Pragma("GCC diagnostic push") \ - _Pragma("GCC diagnostic ignored \"-Wincompatible-pointer-types-discards-qualifiers\"") \ - _G_DEFINE_TYPE_EXTENDED_BEGIN_PRE(TypeName, type_name, TYPE_PARENT) \ - _G_DEFINE_TYPE_EXTENDED_BEGIN_REGISTER(TypeName, type_name, TYPE_PARENT, flags) \ - _Pragma("GCC diagnostic pop") - -# endif /* __clang__ */ - -#else /* GLib < 2.67.0 */ +#if !GLIB_CHECK_VERSION(2, 67, 0) /* * ...meanwhile GCC >= 11 has started issuing warnings about volatile -- 2.45.2

2 1

[PATCH] add support for xml about cipher info
by luzhipeng 12 Aug '24

12 Aug '24

The destination disk's xml contains the ciper info and it check fail, when snaphot,blockcopy and blockpull vm's disk. So it removes the check about cipher info. Signed-off-by: luzhipeng <luzhipeng(a)cestc.cn> --- src/conf/domain_validate.c | 12 ------------ 1 file changed, 12 deletions(-) diff --git a/src/conf/domain_validate.c b/src/conf/domain_validate.c index 39b8d67928..b70edcaaa0 100644 --- a/src/conf/domain_validate.c +++ b/src/conf/domain_validate.c @@ -529,18 +529,6 @@ virDomainDiskDefValidateSourceChainOne(const virStorageSource *src) } } - if (src->encryption) { - virStorageEncryption *encryption = src->encryption; - - if (encryption->format == VIR_STORAGE_ENCRYPTION_FORMAT_LUKS && - encryption->encinfo.cipher_name) { - - virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", - _("supplying <cipher> for domain disk definition is unnecessary")); - return -1; - } - } - /* internal snapshots and config files are currently supported only with rbd: */ if (virStorageSourceGetActualType(src) != VIR_STORAGE_TYPE_NETWORK && src->protocol != VIR_STORAGE_NET_PROTOCOL_RBD) { -- 2.39.3

3 2

[PATCH] crypto: add support for sm4 without key length suffix
by luzhipeng 12 Aug '24

12 Aug '24

qemu add sm4 in release 9, but the name of sm4 doesn't have the key length suffix, So set size to 0, construct cipher name without key length as suffix. Signed-off-by: luzhipeng <luzhipeng(a)cestc.cn> --- docs/formatstorageencryption.rst | 8 +++++--- src/qemu/qemu_block.c | 10 +++++++--- 2 files changed, 12 insertions(+), 6 deletions(-) diff --git a/docs/formatstorageencryption.rst b/docs/formatstorageencryption.rst index 066d285090..6cb8cf024c 100644 --- a/docs/formatstorageencryption.rst +++ b/docs/formatstorageencryption.rst @@ -75,11 +75,13 @@ initialization vector generation. ``name`` The name of the cipher algorithm used for data encryption, such as 'aes', - 'des', 'cast5', 'serpent', 'twofish', etc. Support of the specific + 'des', 'cast5', 'serpent', 'twofish', 'sm4', etc. Support of the specific algorithm is storage driver implementation dependent. ``size`` - The size of the cipher in bits, such as '256', '192', '128', etc. Support - of the specific size for a specific cipher is hypervisor dependent. + The size of the cipher in bits, such as '256', '192', '128', '0', etc. + '0' indicates that the encryption algorithm name doesn't have key length + suffix. Support of the specific size for a specific cipher is hypervisor + dependent. ``mode`` An optional cipher algorithm mode such as 'cbc', 'xts', 'ecb', etc. Support of the specific cipher mode is hypervisor dependent. diff --git a/src/qemu/qemu_block.c b/src/qemu/qemu_block.c index d6cdf521c4..f63f3a36e0 100644 --- a/src/qemu/qemu_block.c +++ b/src/qemu/qemu_block.c @@ -2137,9 +2137,13 @@ qemuBlockStorageSourceCreateGetEncryptionLUKS(virStorageSource *src, if (src->encryption) { if (src->encryption->encinfo.cipher_name) { - cipheralg = g_strdup_printf("%s-%u", - src->encryption->encinfo.cipher_name, - src->encryption->encinfo.cipher_size); + if (src->encryption->encinfo.cipher_size) { + cipheralg = g_strdup_printf("%s-%u", + src->encryption->encinfo.cipher_name, + src->encryption->encinfo.cipher_size); + } else { + cipheralg = g_strdup_printf("%s", src->encryption->encinfo.cipher_name); + } } if (virJSONValueObjectAdd(&props, -- 2.31.1

1 0

[PATCH v2 0/6] qemu: Introduce the ability to disable the built-in PS/2 controller
by Kamil Szczęk 11 Aug '24

11 Aug '24

A while back QEMU introduced a new machine property for disabling the i8042 PS/2 controller (commit 4ccd5fe22feb95137d325f422016a6473541fe9f) which up until then was a built-in device included by all PC machine type descendants unconditionally. This new option allowed users to disable emulation of this controller, thus removing the default PS/2 peripherals. The rationale for why somebody might want to disable PS/2 peripherals is explained in the aforementioned commit. This series of patches exposes this machine property via the 'ps2' feature, while also taking care of side-effects, such as inhibiting the implicit creation of PS/2 inputs in the domain XML. Changes from v1: - Use abstract 'generic-pc' machine type instead of concrete 'pc' for property detection. - Introduce test cases along with monitor replies and capability flags. - Add NEWS mention of the new feature. Kamil Szczęk (6): qemu: Improve PS/2 controller detection qemu_capabilities: Introduce QEMU_CAPS_MACHINE_I8042_OPT qemucapabilitiesdata: Add data for QEMU_CAPS_MACHINE_I8042_OPT qemu: Introduce 'ps2' feature qemuxmlconftest: Add test cases for the new 'ps2' feature NEWS: Mention the new 'ps2' feature NEWS.rst | 6 + docs/formatdomain.rst | 5 + src/conf/domain_conf.c | 6 +- src/conf/domain_conf.h | 1 + src/conf/domain_validate.c | 23 ++ src/conf/schemas/domaincommon.rng | 5 + src/qemu/qemu_capabilities.c | 42 +++ src/qemu/qemu_capabilities.h | 9 +- src/qemu/qemu_command.c | 5 + src/qemu/qemu_domain.c | 29 ++- src/qemu/qemu_domain.h | 1 + src/qemu/qemu_validate.c | 27 +- .../caps_5.2.0_x86_64.replies | 169 ++++++++++-- .../caps_6.0.0_x86_64.replies | 189 ++++++++++++-- .../caps_6.1.0_x86_64.replies | 203 +++++++++++++-- .../caps_6.2.0_x86_64.replies | 212 ++++++++++++++-- .../caps_7.0.0_x86_64.replies | 225 ++++++++++++++-- .../caps_7.0.0_x86_64.xml | 1 + .../caps_7.1.0_x86_64.replies | 240 ++++++++++++++++-- .../caps_7.1.0_x86_64.xml | 1 + .../caps_7.2.0_x86_64+hvf.replies | 240 ++++++++++++++++-- .../caps_7.2.0_x86_64+hvf.xml | 1 + .../caps_7.2.0_x86_64.replies | 240 ++++++++++++++++-- .../caps_7.2.0_x86_64.xml | 1 + .../caps_8.0.0_x86_64.replies | 240 ++++++++++++++++-- .../caps_8.0.0_x86_64.xml | 1 + .../caps_8.1.0_x86_64.replies | 236 +++++++++++++++-- .../caps_8.1.0_x86_64.xml | 1 + .../caps_8.2.0_x86_64.replies | 236 +++++++++++++++-- .../caps_8.2.0_x86_64.xml | 1 + .../caps_9.0.0_x86_64.replies | 240 ++++++++++++++++-- .../caps_9.0.0_x86_64.xml | 1 + .../caps_9.1.0_x86_64.replies | 240 ++++++++++++++++-- .../caps_9.1.0_x86_64.xml | 1 + ...-off-explicit-ps2-inputs.x86_64-latest.err | 1 + .../machine-i8042-off-explicit-ps2-inputs.xml | 19 ++ ...hine-i8042-off-vmport-on.x86_64-latest.err | 1 + .../machine-i8042-off-vmport-on.xml | 18 ++ .../machine-i8042-off.x86_64-6.2.0.err | 1 + .../machine-i8042-off.x86_64-latest.args | 33 +++ .../machine-i8042-off.x86_64-latest.xml | 32 +++ tests/qemuxmlconfdata/machine-i8042-off.xml | 17 ++ .../machine-i8042-on.x86_64-6.2.0.err | 1 + .../machine-i8042-on.x86_64-latest.args | 33 +++ .../machine-i8042-on.x86_64-latest.xml | 34 +++ tests/qemuxmlconfdata/machine-i8042-on.xml | 17 ++ tests/qemuxmlconftest.c | 6 + 47 files changed, 3030 insertions(+), 261 deletions(-) create mode 100644 tests/qemuxmlconfdata/machine-i8042-off-explicit-ps2-inputs.x86_64-latest.err create mode 100644 tests/qemuxmlconfdata/machine-i8042-off-explicit-ps2-inputs.xml create mode 100644 tests/qemuxmlconfdata/machine-i8042-off-vmport-on.x86_64-latest.err create mode 100644 tests/qemuxmlconfdata/machine-i8042-off-vmport-on.xml create mode 100644 tests/qemuxmlconfdata/machine-i8042-off.x86_64-6.2.0.err create mode 100644 tests/qemuxmlconfdata/machine-i8042-off.x86_64-latest.args create mode 100644 tests/qemuxmlconfdata/machine-i8042-off.x86_64-latest.xml create mode 100644 tests/qemuxmlconfdata/machine-i8042-off.xml create mode 100644 tests/qemuxmlconfdata/machine-i8042-on.x86_64-6.2.0.err create mode 100644 tests/qemuxmlconfdata/machine-i8042-on.x86_64-latest.args create mode 100644 tests/qemuxmlconfdata/machine-i8042-on.x86_64-latest.xml create mode 100644 tests/qemuxmlconfdata/machine-i8042-on.xml -- 2.45.0

1 6

[PATCH v2] vmx: Ensure unique disk targets when parsing
by Adam Julis 09 Aug '24

09 Aug '24

Disk targets are generated in virVMXParseConfig() with virVMXGenerateDiskTarget(). It works on combination of controller, fix offset, unit and prefix. While SCSI and SATA have same prefix "sd", function virVMXGenerateDiskTarget() could returned in some cases same targets. In this patch, after loaded SCSI and SATA disks to the def, indexes are regenerated, now simply from position of the disk in array of disks (def). With this, required uniqueness is guaranteed. Because assigned addresses of disks are generated from their indexes, for every changed SATA disk is called virDomainDiskDefAssignAddress() with the updated value. The corresponding tests have been modified to match the index changes. Signed-off-by: Adam Julis <ajulis(a)redhat.com> --- Since previous version in mailing list was complicated for trying to preserve the indexes of SCSI and previous tests, this one going to straightforward, although it changes all (SCSI and SATA) indexes. It's not a bug, since we cannot guarantee the same naming inside the guest anyway. src/vmx/vmx.c | 19 +++++++++++++++++++ tests/vmx2xmldata/esx-in-the-wild-11.xml | 4 ++-- tests/vmx2xmldata/esx-in-the-wild-12.xml | 4 ++-- tests/vmx2xmldata/esx-in-the-wild-2.xml | 4 ++-- tests/vmx2xmldata/esx-in-the-wild-8.xml | 4 ++-- tests/vmx2xmldata/scsi-driver.xml | 12 ++++++------ 6 files changed, 33 insertions(+), 14 deletions(-) diff --git a/src/vmx/vmx.c b/src/vmx/vmx.c index 227744d062..22e59726c8 100644 --- a/src/vmx/vmx.c +++ b/src/vmx/vmx.c @@ -1400,6 +1400,7 @@ virVMXParseConfig(virVMXContext *ctx, virCPUDef *cpu = NULL; char *firmware = NULL; size_t saved_ndisks = 0; + size_t i; if (ctx->parseFileName == NULL) { virReportError(VIR_ERR_INTERNAL_ERROR, "%s", @@ -1805,6 +1806,24 @@ virVMXParseConfig(virVMXContext *ctx, } } + /* now disks contain only SCSI and SATA, SATA could have same name (dst) as SCSI + * so replace all their names with new ones to guarantee their uniqueness + * finally, regenerate correct addresses, while it depends on the index */ + + for (i = 0; i < def->ndisks; i++) { + virDomainDiskDef *disk = def->disks[i]; + + VIR_FREE(disk->dst); + disk->dst = virIndexToDiskName(i, "sd"); + + if (virDomainDiskDefAssignAddress(NULL, disk, def) < 0) { + virReportError(VIR_ERR_INTERNAL_ERROR, + _("Could not assign address to disk '%1$s'"), + virDomainDiskGetSource(disk)); + goto cleanup; + } + } + /* def:disks (ide) */ for (bus = 0; bus < 2; ++bus) { for (unit = 0; unit < 2; ++unit) { diff --git a/tests/vmx2xmldata/esx-in-the-wild-11.xml b/tests/vmx2xmldata/esx-in-the-wild-11.xml index 8807a057d7..d9522c1be2 100644 --- a/tests/vmx2xmldata/esx-in-the-wild-11.xml +++ b/tests/vmx2xmldata/esx-in-the-wild-11.xml @@ -22,8 +22,8 @@ </disk> <disk type='file' device='disk'> <source file='[datastore] directory/esx6.7-rhel7.7-x86_64_3.vmdk'/> - <target dev='sdp' bus='scsi'/> - <address type='drive' controller='0' bus='0' target='0' unit='16'/> + <target dev='sdb' bus='scsi'/> + <address type='drive' controller='0' bus='0' target='0' unit='1'/> </disk> <controller type='scsi' index='0' model='vmpvscsi'/> <interface type='bridge'> diff --git a/tests/vmx2xmldata/esx-in-the-wild-12.xml b/tests/vmx2xmldata/esx-in-the-wild-12.xml index 42184501d0..a7730845ee 100644 --- a/tests/vmx2xmldata/esx-in-the-wild-12.xml +++ b/tests/vmx2xmldata/esx-in-the-wild-12.xml @@ -21,9 +21,9 @@ <address type='drive' controller='0' bus='0' target='0' unit='0'/> </disk> <disk type='file' device='cdrom'> - <target dev='sda' bus='sata'/> + <target dev='sdb' bus='sata'/> <readonly/> - <address type='drive' controller='0' bus='0' target='0' unit='0'/> + <address type='drive' controller='0' bus='0' target='0' unit='1'/> </disk> <controller type='scsi' index='0' model='vmpvscsi'/> <controller type='sata' index='0'/> diff --git a/tests/vmx2xmldata/esx-in-the-wild-2.xml b/tests/vmx2xmldata/esx-in-the-wild-2.xml index 59071b5d3a..1a66f5e9c7 100644 --- a/tests/vmx2xmldata/esx-in-the-wild-2.xml +++ b/tests/vmx2xmldata/esx-in-the-wild-2.xml @@ -20,9 +20,9 @@ </disk> <disk type='file' device='cdrom'> <source file='[datastore] directory/Debian1-cdrom.iso'/> - <target dev='sdp' bus='scsi'/> + <target dev='sdb' bus='scsi'/> <readonly/> - <address type='drive' controller='1' bus='0' target='0' unit='0'/> + <address type='drive' controller='0' bus='0' target='0' unit='1'/> </disk> <disk type='file' device='cdrom'> <source file='/vmimages/tools-isoimages/linux.iso'/> diff --git a/tests/vmx2xmldata/esx-in-the-wild-8.xml b/tests/vmx2xmldata/esx-in-the-wild-8.xml index 47d22ced2a..d5356bda34 100644 --- a/tests/vmx2xmldata/esx-in-the-wild-8.xml +++ b/tests/vmx2xmldata/esx-in-the-wild-8.xml @@ -36,9 +36,9 @@ </disk> <disk type='file' device='cdrom'> <source file='[692eb778-2d4937fe] CentOS-4.7.ServerCD-x86_64.iso'/> - <target dev='sda' bus='sata'/> + <target dev='sdd' bus='sata'/> <readonly/> - <address type='drive' controller='0' bus='0' target='0' unit='0'/> + <address type='drive' controller='0' bus='0' target='0' unit='3'/> </disk> <controller type='scsi' index='0' model='vmpvscsi'/> <controller type='sata' index='0'/> diff --git a/tests/vmx2xmldata/scsi-driver.xml b/tests/vmx2xmldata/scsi-driver.xml index e5b73420c3..42b6fffe24 100644 --- a/tests/vmx2xmldata/scsi-driver.xml +++ b/tests/vmx2xmldata/scsi-driver.xml @@ -19,18 +19,18 @@ </disk> <disk type='file' device='disk'> <source file='[datastore] directory/harddisk2.vmdk'/> - <target dev='sdp' bus='scsi'/> - <address type='drive' controller='1' bus='0' target='0' unit='0'/> + <target dev='sdb' bus='scsi'/> + <address type='drive' controller='0' bus='0' target='0' unit='1'/> </disk> <disk type='file' device='disk'> <source file='[datastore] directory/harddisk3.vmdk'/> - <target dev='sdae' bus='scsi'/> - <address type='drive' controller='2' bus='0' target='0' unit='0'/> + <target dev='sdc' bus='scsi'/> + <address type='drive' controller='0' bus='0' target='0' unit='2'/> </disk> <disk type='file' device='disk'> <source file='[datastore] directory/harddisk4.vmdk'/> - <target dev='sdat' bus='scsi'/> - <address type='drive' controller='3' bus='0' target='0' unit='0'/> + <target dev='sdd' bus='scsi'/> + <address type='drive' controller='0' bus='0' target='0' unit='3'/> </disk> <controller type='scsi' index='0' model='buslogic'/> <controller type='scsi' index='1' model='lsilogic'/> -- 2.45.2

2 1

[PATCH v2 0/3] qemuxmlconftest: Add test cases for broken and missing XML files
by Peter Krempa 09 Aug '24

09 Aug '24

Patch 3/3 adds the actual test: Changes to v1: - fixed missing NULL termination of the 'skip' array - "backported" g_string_replace into glibcompat.c until we bump minimum glib - started striping 'abs_srcdir' strings from qemuxmlconftest's '.err' output files Peter Krempa (3): glibcompat: "Backport" 'g_string_replace' testQemuConfXMLCommon: Strip 'abs_srcdir' paths from '.err' files in qemuxmlconftest qemuxmlconftest: Add test cases for broken and missing XML files src/libvirt_private.syms | 1 + src/util/glibcompat.c | 65 +++++++++++++++++++ src/util/glibcompat.h | 10 +++ .../broken-xml-invalid.x86_64-latest.err | 3 + tests/qemuxmlconfdata/broken-xml-invalid.xml | 1 + .../nonexistent-file.x86_64-latest.err | 1 + tests/qemuxmlconftest.c | 16 ++++- tests/testutilsqemu.h | 1 + tests/virschematest.c | 12 ++++ 9 files changed, 107 insertions(+), 3 deletions(-) create mode 100644 tests/qemuxmlconfdata/broken-xml-invalid.x86_64-latest.err create mode 100644 tests/qemuxmlconfdata/broken-xml-invalid.xml create mode 100644 tests/qemuxmlconfdata/nonexistent-file.x86_64-latest.err -- 2.45.2

2 5