[PATCH] virsh: snapshot: Don't validate schema of XML generated by 'virsh snapshot-create-as'
by Peter Krempa
Commit 95f8e3237e5486f487324c6 which introduced XML schema validation
for snapshot XMLs always asserted the validation for the XML generated
by 'virsh snapshot-create-as' on the basis that it's libvirt-generated,
thus valid.
This unfortunately isn't true as users can influence certain bits of the
XML such as the disk image path which must be a full path. Thus if a
user tries to invoke virsh as:
$ virsh snapshot-create-as upstream --diskspec vda,file=relative.qcow2
error: XML document failed to validate against schema: Unable to validate doc against /path/to/domainsnapshot.rng
Extra element disks in interleave
Element domainsnapshot failed to validate content
They get a rather useless error from the libxml2 RNG validator.
With this fix applied, we get to the XML parser in libvirtd which has a
more reasonable error:
$ virsh snapshot-create-as upstream --diskspec vda,file=relative.qcow2
error: XML error: disk snapshot image path 'relative.qcow2' must be absolute
Instead users can force validation of the XML generated by 'virsh
snapshot-create-as' by passing the '--validate' flag.
Signed-off-by: Peter Krempa <pkrempa(a)redhat.com>
---
docs/manpages/virsh.rst | 4 +++-
tools/virsh-snapshot.c | 8 +++++++-
2 files changed, 10 insertions(+), 2 deletions(-)
diff --git a/docs/manpages/virsh.rst b/docs/manpages/virsh.rst
index bccda292a2..ad91cd6356 100644
--- a/docs/manpages/virsh.rst
+++ b/docs/manpages/virsh.rst
@@ -6916,7 +6916,7 @@ snapshot-create-as
snapshot-create-as domain {[--print-xml] [--no-metadata]
[--halt] [--reuse-external]} [name]
- [description] [--disk-only [--quiesce]] [--atomic]
+ [description] [--disk-only [--quiesce]] [--atomic] [--validate]
[[--live] [--memspec memspec]] [--diskspec] diskspec]...
Create a snapshot for domain *domain* with the given <name> and
@@ -6988,6 +6988,8 @@ For now, it is not possible to create snapshots in a domain that has
checkpoints, although this restriction will be lifted in a future
release.
+Optionally, the *--validate* option can be passed to validate XML document
+which is internally generated by this command against the internal RNG schema.
snapshot-current
----------------
diff --git a/tools/virsh-snapshot.c b/tools/virsh-snapshot.c
index 2bc2cb7e23..2bec722c61 100644
--- a/tools/virsh-snapshot.c
+++ b/tools/virsh-snapshot.c
@@ -372,6 +372,10 @@ static const vshCmdOptDef opts_snapshot_create_as[] = {
.help = N_("require atomic operation")
},
VIRSH_COMMON_OPT_LIVE(N_("take a live snapshot")),
+ {.name = "validate",
+ .type = VSH_OT_BOOL,
+ .help = N_("validate the XML against the schema"),
+ },
{.name = "memspec",
.type = VSH_OT_STRING,
.flags = VSH_OFLAG_REQ_OPT,
@@ -394,7 +398,7 @@ cmdSnapshotCreateAs(vshControl *ctl, const vshCmd *cmd)
const char *desc = NULL;
const char *memspec = NULL;
g_auto(virBuffer) buf = VIR_BUFFER_INITIALIZER;
- unsigned int flags = VIR_DOMAIN_SNAPSHOT_CREATE_VALIDATE;
+ unsigned int flags = 0;
const vshCmdOpt *opt = NULL;
if (vshCommandOptBool(cmd, "no-metadata"))
@@ -411,6 +415,8 @@ cmdSnapshotCreateAs(vshControl *ctl, const vshCmd *cmd)
flags |= VIR_DOMAIN_SNAPSHOT_CREATE_ATOMIC;
if (vshCommandOptBool(cmd, "live"))
flags |= VIR_DOMAIN_SNAPSHOT_CREATE_LIVE;
+ if (vshCommandOptBool(cmd, "validate"))
+ flags |= VIR_DOMAIN_SNAPSHOT_CREATE_VALIDATE;
if (!(dom = virshCommandOptDomain(ctl, cmd, NULL)))
return false;
--
2.30.2
3 years, 6 months
[libvirt PATCH v2 0/2] Fix some typos and spelling mistakes
by Tim Wiederhake
Found by codespell [1].
It would be nice to automate this, but I could not get codespell to
also check code comments, and ran into a known issue [2] with codespell
that prevents putting some terms with mixed letter cases on the skip list.
[1] https://github.com/codespell-project/codespell
[2] https://github.com/codespell-project/codespell/issues/1860
V1: https://listman.redhat.com/archives/libvir-list/2021-April/msg00562.html
Changes since V1:
* Added "Fixes:" to patch #1 commit message
* Removed "crashes->crashers", "preferable->preferrable", and
"purposedly->purposely" from patch #2
Tim Wiederhake (2):
Fix spelling
virlog: Remove stray "todo" in comment
NEWS.rst | 10 +++++-----
docs/formatdomain.rst | 6 +++---
docs/kbase/launch_security_sev.rst | 2 +-
docs/manpages/virtproxyd.rst | 2 +-
docs/pci-addresses.rst | 2 +-
src/hyperv/hyperv_wmi.c | 2 +-
src/libvirt-domain.c | 2 +-
src/qemu/qemu_capabilities.c | 2 +-
src/qemu/qemu_command.c | 2 +-
src/qemu/qemu_domain.c | 4 ++--
src/qemu/qemu_domain.h | 4 ++--
src/qemu/qemu_interface.c | 2 +-
src/qemu/qemu_snapshot.c | 4 ++--
src/util/virerror.c | 3 ++-
src/util/virhash.c | 2 +-
src/util/virlog.c | 2 +-
src/util/virnetdev.c | 2 +-
src/util/virpci.c | 2 +-
src/util/virpci.h | 2 +-
tests/meson.build | 2 +-
tools/meson.build | 2 +-
21 files changed, 31 insertions(+), 30 deletions(-)
--
2.26.2
3 years, 6 months
[PATCH] vbox: Make host CPU cache init non-fatal
by Charlie Sharpsteen
This commit updates the vbox driver to log a warning if the
virCapabilitiesInitCaches function fails instead of treating failure
as a fatal error. CPU cache initialization requires the sysfs filesystem
in order to complete successfully and thus fails on platforms such as macOS.
This commit is similar to ace6528ae19, which made the same change to the
VMware driver.
Signed-off-by: Charlie Sharpsteen <source(a)sharpsteen.net>
---
src/vbox/vbox_common.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/src/vbox/vbox_common.c b/src/vbox/vbox_common.c
index 5a5de85eab..933851346b 100644
--- a/src/vbox/vbox_common.c
+++ b/src/vbox/vbox_common.c
@@ -98,7 +98,7 @@ vboxCapsInit(void)
return NULL;
if (virCapabilitiesInitCaches(caps) < 0)
- return NULL;
+ VIR_WARN("Failed to get host CPU cache info");
if ((guest = virCapabilitiesAddGuest(caps,
VIR_DOMAIN_OSTYPE_HVM,
--
2.31.1
3 years, 6 months
[libvirt PATCH 0/2] Always validate XML for (hypervisor-)cpu-compare
by Tim Wiederhake
XML schema validation for `virsh (hypervisor-)cpu-compare` has to be
enabled explicitly by passing the `--validate` flag. Having invalid
XML domain / cpu specification that appear to work can lead to hard
to find problems down the line, when e.g. migration of VMs does not
work as expected.
This series fixes a bug in the validation code and logs the schema
validation error to libvirtd's log file. User facing behaviour stays
unchanged.
See this conversation for more background:
https://listman.redhat.com/archives/libvir-list/2021-March/msg01214.html
Tim Wiederhake (2):
virxml: Fix schema validation of individual nodes
virCPUDefParseXML: Log schema validation errors
src/conf/cpu_conf.c | 16 +++++++---------
src/util/virxml.c | 13 ++++++-------
src/util/virxml.h | 1 -
3 files changed, 13 insertions(+), 17 deletions(-)
--
2.26.2
3 years, 6 months
[PATCH v2 0/2] qemu: virtiofs: support <sandbox mode='chroot'/>
by Cole Robinson
Add support for virtiofsd -o sandbox=chroot|namespace
v2:
Add link to virtiofsd docs in libvirt docs
validate the new field for qemu
break up the patch
Cole Robinson (2):
conf: Introduce <sandbox mode='chroot'/> for <filesystem><binary>
qemu: virtiofs: support <sandbox mode='chroot'/>
docs/formatdomain.rst | 6 +++++
docs/schemas/domaincommon.rng | 12 ++++++++++
src/conf/domain_conf.c | 23 +++++++++++++++++++
src/conf/domain_conf.h | 10 ++++++++
src/libvirt_private.syms | 1 +
src/qemu/qemu_validate.c | 7 ++++++
src/qemu/qemu_virtiofs.c | 2 ++
.../vhost-user-fs-fd-memory.xml | 1 +
8 files changed, 62 insertions(+)
--
2.30.2
3 years, 6 months
[libvirt PATCH v2 1/3] qemu: remove unnecessary null check
by Jonathon Jongsma
virMediatedDeviceGetSysfsPath() (via g_strdup_printf()) is guaranteed to
return a non-NULL value, so remove the unnecessary checks for NULL.
Signed-off-by: Jonathon Jongsma <jjongsma(a)redhat.com>
Reviewed-by: Laine Stump <laine(a)redhat.com>
---
Changes in v2:
- remove NULL checks from 2 additional locations
src/qemu/qemu_command.c | 4 +---
src/util/virmdev.c | 7 +------
2 files changed, 2 insertions(+), 9 deletions(-)
diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
index 83bebdd2a8..fbe387253e 100644
--- a/src/qemu/qemu_command.c
+++ b/src/qemu/qemu_command.c
@@ -5138,9 +5138,7 @@ qemuBuildHostdevMediatedDevStr(const virDomainDef *def,
g_autofree char *mdevPath = NULL;
const char *dev_str = NULL;
- if (!(mdevPath = virMediatedDeviceGetSysfsPath(mdevsrc->uuidstr)))
- return NULL;
-
+ mdevPath = virMediatedDeviceGetSysfsPath(mdevsrc->uuidstr);
dev_str = qemuBuildHostdevMdevModelTypeString(mdevsrc);
if (!dev_str)
diff --git a/src/util/virmdev.c b/src/util/virmdev.c
index 7f4a499536..5f112c775f 100644
--- a/src/util/virmdev.c
+++ b/src/util/virmdev.c
@@ -140,9 +140,7 @@ virMediatedDeviceNew(const char *uuidstr, virMediatedDeviceModelType model)
g_autoptr(virMediatedDevice) dev = NULL;
g_autofree char *sysfspath = NULL;
- if (!(sysfspath = virMediatedDeviceGetSysfsPath(uuidstr)))
- return NULL;
-
+ sysfspath = virMediatedDeviceGetSysfsPath(uuidstr);
if (!virFileExists(sysfspath)) {
virReportError(VIR_ERR_DEVICE_MISSING,
_("mediated device '%s' not found"), uuidstr);
@@ -208,9 +206,6 @@ virMediatedDeviceGetIOMMUGroupDev(const char *uuidstr)
g_autofree char *iommu_path = NULL;
g_autofree char *dev_path = virMediatedDeviceGetSysfsPath(uuidstr);
- if (!dev_path)
- return NULL;
-
iommu_path = g_strdup_printf("%s/iommu_group", dev_path);
if (!virFileExists(iommu_path)) {
--
2.26.3
3 years, 6 months
[libvirt PATCH 0/2] Fix some typos and spelling mistakes
by Tim Wiederhake
Found by codespell [1].
It would be nice to automate this, but I could not get codespell to
also check code comments, and ran into a known issue [2] with codespell
that prevents putting some terms with mixed letter cases on the skip list.
[1] https://github.com/codespell-project/codespell
[2] https://github.com/codespell-project/codespell/issues/1860
Tim Wiederhake (2):
Fix spelling
virlog: Remove stray "todo" in comment
NEWS.rst | 12 ++++++------
docs/formatdomain.rst | 6 +++---
docs/glib-adoption.rst | 2 +-
docs/kbase/launch_security_sev.rst | 2 +-
docs/manpages/virtproxyd.rst | 2 +-
docs/pci-addresses.rst | 2 +-
src/hyperv/hyperv_wmi.c | 2 +-
src/libvirt-domain.c | 2 +-
src/libxl/libxl_logger.c | 2 +-
src/qemu/qemu_capabilities.c | 2 +-
src/qemu/qemu_command.c | 2 +-
src/qemu/qemu_domain.c | 4 ++--
src/qemu/qemu_domain.h | 4 ++--
src/qemu/qemu_interface.c | 2 +-
src/qemu/qemu_snapshot.c | 4 ++--
src/util/virerror.c | 3 ++-
src/util/virhash.c | 2 +-
src/util/virlog.c | 2 +-
src/util/virnetdev.c | 2 +-
src/util/virpci.c | 2 +-
src/util/virpci.h | 2 +-
tests/meson.build | 2 +-
tools/meson.build | 2 +-
23 files changed, 34 insertions(+), 33 deletions(-)
--
2.26.2
3 years, 6 months
[PATCH v2] qemu: Expose disk serial in virDomainGetGuestInfo()
by Michal Privoznik
When querying guest info via virDomainGetGuestInfo() the
'guest-get-disks' agent command is called. It may report disk
serial number which we parse, but never report nor use for
anything else.
As it turns out, it may help management application find matching
disk in their internals.
Signed-off-by: Michal Privoznik <mprivozn(a)redhat.com>
---
v2 of:
https://listman.redhat.com/archives/libvir-list/2021-April/msg00552.html
diff to v1:
- Filled virsh documentation
- Documented that the param is optional and type of string
docs/manpages/virsh.rst | 1 +
src/libvirt-domain.c | 1 +
src/qemu/qemu_driver.c | 19 ++++++++++++++-----
3 files changed, 16 insertions(+), 5 deletions(-)
diff --git a/docs/manpages/virsh.rst b/docs/manpages/virsh.rst
index 813fb0bd60..bccda292a2 100644
--- a/docs/manpages/virsh.rst
+++ b/docs/manpages/virsh.rst
@@ -2787,6 +2787,7 @@ returned:
* ``disk.<num>.partition`` - whether this is a partition or disk
* ``disk.<num>.dependency.count`` - the number of device dependencies
* ``disk.<num>.dependency.<num>.name`` - a dependency name
+* ``disk.<num>.serial`` - optional disk serial number
* ``disk.<num>.alias`` - the device alias of the disk (e.g. sda)
* ``disk.<num>.guest_alias`` - optional alias assigned to the disk
diff --git a/src/libvirt-domain.c b/src/libvirt-domain.c
index 83ff7df9fe..509b7a6d88 100644
--- a/src/libvirt-domain.c
+++ b/src/libvirt-domain.c
@@ -12561,6 +12561,7 @@ virDomainSetVcpu(virDomainPtr domain,
* hold the list of PVs, for LUKS encrypted volume this will
* contain the disk where the volume is placed. (Linux)
* "disk.<num>.dependency.<num>.name" - a dependency
+ * "disk.<num>.serial" - optional disk serial number (as string)
* "disk.<num>.alias" - the device alias of the disk (e.g. sda)
* "disk.<num>.guest_alias" - optional alias assigned to the disk, on Linux
* this is a name assigned by device mapper
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index 90b5477f00..88ee9e5d5e 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -19951,15 +19951,24 @@ qemuAgentDiskInfoFormatParams(qemuAgentDiskInfo **info,
}
if (info[i]->address) {
+ qemuAgentDiskAddress *address = info[i]->address;
virDomainDiskDef *diskdef = NULL;
+ if (address->serial) {
+ g_snprintf(param_name, VIR_TYPED_PARAM_FIELD_LENGTH,
+ "disk.%zu.serial", i);
+ if (virTypedParamsAddString(params, nparams, maxparams,
+ param_name, address->serial) < 0)
+ return;
+ }
+
/* match the disk to the target in the vm definition */
diskdef = virDomainDiskByAddress(vmdef,
- &info[i]->address->pci_controller,
- info[i]->address->ccw_addr,
- info[i]->address->bus,
- info[i]->address->target,
- info[i]->address->unit);
+ &address->pci_controller,
+ address->ccw_addr,
+ address->bus,
+ address->target,
+ address->unit);
if (diskdef) {
g_snprintf(param_name, VIR_TYPED_PARAM_FIELD_LENGTH,
"disk.%zu.alias", i);
--
2.26.3
3 years, 6 months
[PATCH] qemu: Expose disk serial in virDomainGetGuestInfo()
by Michal Privoznik
When querying guest info via virDomainGetGuestInfo() the
'guest-get-disks' agent command is called. It may report disk
serial number which we parse, but never report nor use for
anything else.
As it turns out, it may help management application find matching
disk in their internals.
Signed-off-by: Michal Privoznik <mprivozn(a)redhat.com>
---
src/libvirt-domain.c | 1 +
src/qemu/qemu_driver.c | 19 ++++++++++++++-----
2 files changed, 15 insertions(+), 5 deletions(-)
diff --git a/src/libvirt-domain.c b/src/libvirt-domain.c
index 83ff7df9fe..1c244da650 100644
--- a/src/libvirt-domain.c
+++ b/src/libvirt-domain.c
@@ -12561,6 +12561,7 @@ virDomainSetVcpu(virDomainPtr domain,
* hold the list of PVs, for LUKS encrypted volume this will
* contain the disk where the volume is placed. (Linux)
* "disk.<num>.dependency.<num>.name" - a dependency
+ * "disk.<num>.serial" - disk serial number
* "disk.<num>.alias" - the device alias of the disk (e.g. sda)
* "disk.<num>.guest_alias" - optional alias assigned to the disk, on Linux
* this is a name assigned by device mapper
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index 90b5477f00..88ee9e5d5e 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -19951,15 +19951,24 @@ qemuAgentDiskInfoFormatParams(qemuAgentDiskInfo **info,
}
if (info[i]->address) {
+ qemuAgentDiskAddress *address = info[i]->address;
virDomainDiskDef *diskdef = NULL;
+ if (address->serial) {
+ g_snprintf(param_name, VIR_TYPED_PARAM_FIELD_LENGTH,
+ "disk.%zu.serial", i);
+ if (virTypedParamsAddString(params, nparams, maxparams,
+ param_name, address->serial) < 0)
+ return;
+ }
+
/* match the disk to the target in the vm definition */
diskdef = virDomainDiskByAddress(vmdef,
- &info[i]->address->pci_controller,
- info[i]->address->ccw_addr,
- info[i]->address->bus,
- info[i]->address->target,
- info[i]->address->unit);
+ &address->pci_controller,
+ address->ccw_addr,
+ address->bus,
+ address->target,
+ address->unit);
if (diskdef) {
g_snprintf(param_name, VIR_TYPED_PARAM_FIELD_LENGTH,
"disk.%zu.alias", i);
--
2.26.3
3 years, 6 months
