[PATCH] qemu: Don't use memory-backend-memfd for NVDIMMs
by Michal Privoznik
If guest is configured to use memfd then the function that build
memory-backend-* part of command line will put
memory-backend-memfd, always. Even for NVDIMMs. This is not
correct, because NVDIMMs need a backing path (usually to a real
host NVDIMM device). Therefore, regardless of memfd being
requested, we have to stick with memory-backend-file.
Signed-off-by: Michal Privoznik <mprivozn(a)redhat.com>
---
src/qemu/qemu_command.c | 3 ++-
.../memfd-memory-numa.x86_64-latest.args | 6 ++++--
tests/qemuxml2argvdata/memfd-memory-numa.xml | 11 +++++++++++
tests/qemuxml2xmltest.c | 3 ++-
4 files changed, 19 insertions(+), 4 deletions(-)
diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
index ea513693f7..0473e7deaa 100644
--- a/src/qemu/qemu_command.c
+++ b/src/qemu/qemu_command.c
@@ -3052,7 +3052,8 @@ qemuBuildMemoryBackendProps(virJSONValue **backendProps,
props = virJSONValueNewObject();
- if (def->mem.source == VIR_DOMAIN_MEMORY_SOURCE_MEMFD) {
+ if (!mem->nvdimmPath &&
+ def->mem.source == VIR_DOMAIN_MEMORY_SOURCE_MEMFD) {
backendType = "memory-backend-memfd";
if (useHugepage) {
diff --git a/tests/qemuxml2argvdata/memfd-memory-numa.x86_64-latest.args b/tests/qemuxml2argvdata/memfd-memory-numa.x86_64-latest.args
index 5e54908666..3b33db3c55 100644
--- a/tests/qemuxml2argvdata/memfd-memory-numa.x86_64-latest.args
+++ b/tests/qemuxml2argvdata/memfd-memory-numa.x86_64-latest.args
@@ -10,13 +10,15 @@ XDG_CONFIG_HOME=/tmp/lib/domain--1-instance-00000092/.config \
-name guest=instance-00000092,debug-threads=on \
-S \
-object '{"qom-type":"secret","id":"masterKey0","format":"raw","file":"/tmp/lib/domain--1-instance-00000092/master-key.aes"}' \
--machine pc-i440fx-2.3,accel=kvm,usb=off,dump-guest-core=off \
+-machine pc-i440fx-2.3,accel=kvm,usb=off,dump-guest-core=off,nvdimm=on \
-cpu qemu64 \
--m 14336 \
+-m size=14680064k,slots=16,maxmem=1099511627776k \
-overcommit mem-lock=off \
-smp 8,sockets=1,dies=1,cores=8,threads=1 \
-object '{"qom-type":"memory-backend-memfd","id":"ram-node0","hugetlb":true,"hugetlbsize":2097152,"share":true,"prealloc":true,"size":15032385536,"host-nodes":[3],"policy":"preferred"}' \
-numa node,nodeid=0,cpus=0-7,memdev=ram-node0 \
+-object '{"qom-type":"memory-backend-file","id":"memnvdimm0","mem-path":"/tmp/nvdimm","share":true,"prealloc":true,"size":536870912,"host-nodes":[3],"policy":"preferred"}' \
+-device nvdimm,node=0,memdev=memnvdimm0,id=nvdimm0,slot=0 \
-uuid 126f2720-6f8e-45ab-a886-ec9277079a67 \
-display none \
-no-user-config \
diff --git a/tests/qemuxml2argvdata/memfd-memory-numa.xml b/tests/qemuxml2argvdata/memfd-memory-numa.xml
index 3f448790a6..d9e1a9f564 100644
--- a/tests/qemuxml2argvdata/memfd-memory-numa.xml
+++ b/tests/qemuxml2argvdata/memfd-memory-numa.xml
@@ -1,6 +1,7 @@
<domain type='kvm'>
<name>instance-00000092</name>
<uuid>126f2720-6f8e-45ab-a886-ec9277079a67</uuid>
+ <maxMemory slots='16' unit='KiB'>1099511627776</maxMemory>
<memory unit='KiB'>14680064</memory>
<currentMemory unit='KiB'>14680064</currentMemory>
<memoryBacking>
@@ -41,5 +42,15 @@
<memballoon model='virtio'>
<address type='pci' domain='0x0000' bus='0x00' slot='0x03' function='0x0'/>
</memballoon>
+ <memory model='nvdimm'>
+ <source>
+ <path>/tmp/nvdimm</path>
+ </source>
+ <target>
+ <size unit='KiB'>523264</size>
+ <node>0</node>
+ </target>
+ <address type='dimm' slot='0'/>
+ </memory>
</devices>
</domain>
diff --git a/tests/qemuxml2xmltest.c b/tests/qemuxml2xmltest.c
index 40e027aaa4..c5005d41a0 100644
--- a/tests/qemuxml2xmltest.c
+++ b/tests/qemuxml2xmltest.c
@@ -1305,7 +1305,8 @@ mymain(void)
DO_TEST("memfd-memory-numa",
QEMU_CAPS_OBJECT_MEMORY_MEMFD,
QEMU_CAPS_OBJECT_MEMORY_MEMFD_HUGETLB,
- QEMU_CAPS_OBJECT_MEMORY_FILE);
+ QEMU_CAPS_OBJECT_MEMORY_FILE,
+ QEMU_CAPS_DEVICE_NVDIMM);
DO_TEST("memfd-memory-default-hugepage",
QEMU_CAPS_OBJECT_MEMORY_MEMFD,
QEMU_CAPS_OBJECT_MEMORY_MEMFD_HUGETLB,
--
2.31.1
3 years, 3 months
[PATCH] virtlockd: Don't report error if lockspace exists
by Jim Fehlig
When the qemu or libxl driver is configured to use lockd and
file_lockspace_dir is set, virtlockd emits an error when libvirtd
is retarted
May 25 15:44:31 virt81 virtlockd[7723]: Requested operation is not
valid: Lockspace for path /data/libvirtd/lockspace already exists
There is really no need to fail when the lockspace already exists,
paricularly since the user is expected to create the lockspace
specified in file_lockspace_dir. Failure to do so will prevent
starting any domains
virsh start test
error: Failed to start domain 'test'
error: Unable to open/create resource /data/libvirtd/lockspace/de22c4bf931e7c48b49e8ca64b477d44e78a51543e534df488b05ccd08ec5caa: No such file or directory
Also, virLockManagerLockDaemonSetupLockspace already has logic to ignore
the error. Since callers are not interested in the error, change
virtlockd to not report or return an error when the specified lockspace
already exists.
Signed-off-by: Jim Fehlig <jfehlig(a)suse.com>
---
In response to my earlier query about this problem
https://listman.redhat.com/archives/libvir-list/2021-May/msg00872.html
src/locking/lock_daemon_dispatch.c | 5 ++---
src/locking/lock_driver_lockd.c | 11 ++---------
2 files changed, 4 insertions(+), 12 deletions(-)
diff --git a/src/locking/lock_daemon_dispatch.c b/src/locking/lock_daemon_dispatch.c
index e8b9832453..13e688f2e2 100644
--- a/src/locking/lock_daemon_dispatch.c
+++ b/src/locking/lock_daemon_dispatch.c
@@ -405,9 +405,8 @@ virLockSpaceProtocolDispatchCreateLockSpace(virNetServer *server G_GNUC_UNUSED,
}
if (virLockDaemonFindLockSpace(lockDaemon, args->path) != NULL) {
- virReportError(VIR_ERR_OPERATION_INVALID,
- _("Lockspace for path %s already exists"),
- args->path);
+ VIR_DEBUG("Lockspace for path %s already exists", args->path);
+ rv = 0;
goto cleanup;
}
diff --git a/src/locking/lock_driver_lockd.c b/src/locking/lock_driver_lockd.c
index 3a7386af30..87afdbfb78 100644
--- a/src/locking/lock_driver_lockd.c
+++ b/src/locking/lock_driver_lockd.c
@@ -281,15 +281,8 @@ static int virLockManagerLockDaemonSetupLockspace(const char *path)
VIR_LOCK_SPACE_PROTOCOL_PROC_CREATE_LOCKSPACE,
0, NULL, NULL, NULL,
(xdrproc_t)xdr_virLockSpaceProtocolCreateLockSpaceArgs, (char*)&args,
- (xdrproc_t)xdr_void, NULL) < 0) {
- if (virGetLastErrorCode() == VIR_ERR_OPERATION_INVALID) {
- /* The lockspace already exists */
- virResetLastError();
- rv = 0;
- } else {
- goto cleanup;
- }
- }
+ (xdrproc_t)xdr_void, NULL) < 0)
+ goto cleanup;
rv = 0;
--
2.31.1
3 years, 3 months
[PATCH] virDomainMachineNameAppendValid: Handle special characters better
by Michal Privoznik
When constructing guest name for machined we have to be very
cautious as machined expects a name that's basically a valid URI.
Therefore, if there's a dot it has to be followed by a letter or
a number. And if there's a sequence of two or more dashes they
should be joined into a single dash. These rules are implemented
in virDomainMachineNameAppendValid(). There's the @skip variable
which is supposed to track whether it is safe to append a dot or
a dash into name. However, the variable is set to false (meaning
it is safe to append a dot or a dash) even if the current
character we are processing is not in the set of allowed
characters (and thus skipped over).
Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1948433
Signed-off-by: Michal Privoznik <mprivozn(a)redhat.com>
---
src/hypervisor/domain_driver.c | 8 ++++----
tests/virsystemdtest.c | 3 +++
2 files changed, 7 insertions(+), 4 deletions(-)
diff --git a/src/hypervisor/domain_driver.c b/src/hypervisor/domain_driver.c
index f7d49bc38a..29e11c0447 100644
--- a/src/hypervisor/domain_driver.c
+++ b/src/hypervisor/domain_driver.c
@@ -63,18 +63,18 @@ virDomainMachineNameAppendValid(virBuffer *buf,
break;
if (*name == '.' || *name == '-') {
- if (!skip)
+ if (!skip) {
virBufferAddChar(buf, *name);
- skip = true;
+ skip = true;
+ }
continue;
}
- skip = false;
-
if (!strchr(HOSTNAME_CHARS, *name))
continue;
virBufferAddChar(buf, *name);
+ skip = false;
}
/* trailing dashes or dots are not allowed */
diff --git a/tests/virsystemdtest.c b/tests/virsystemdtest.c
index f1c9a4ee5f..a09b428a8a 100644
--- a/tests/virsystemdtest.c
+++ b/tests/virsystemdtest.c
@@ -736,6 +736,9 @@ mymain(void)
TEST_MACHINE("demo.-.test.", NULL, 11, "qemu-11-demo.test");
TEST_MACHINE("demo", "/tmp/root1", 1, "qemu-embed-0991f456-1-demo");
TEST_MACHINE("demo", "/tmp/root2", 1, "qemu-embed-95d47ff5-1-demo");
+ TEST_MACHINE("|.-m", NULL, 1, "qemu-1-m");
+ TEST_MACHINE("Auto-esx7.0-rhel7.9-special-characters~!@#$%^&*_=+,?><:;|.\"[]()`\\-m",
+ NULL, 1, "qemu-1-Auto-esx7.0-rhel7.9-special-characters.m");
# define TESTS_PM_SUPPORT_HELPER(name, function) \
do { \
--
2.31.1
3 years, 3 months
[PATCH v3 0/6] Support for launchSecurity type s390-pv
by Boris Fiuczynski
This patch series introduces the launch security type s390-pv.
Specifying s390-pv as launch security type in an s390 domain prepares for
running the guest in protected virtualization secure mode, also known as
IBM Secure Execution.
diff to v2:
- Broke up previous patch one into three patches
diff to v1:
- Rebased to current master
- Added verification check for confidential-guest-support capability
Boris Fiuczynski (6):
schemas: Make SEV policy on launch security optional
conf: modernize SEV XML parse and format methods
conf: refactor launch security to allow more types
qemu: add s390-pv-guest capability
conf: add s390-pv as launch security type
docs: add s390-pv documentation
docs/formatdomain.rst | 7 +
docs/kbase/s390_protected_virt.rst | 55 ++++++-
docs/schemas/domaincommon.rng | 13 +-
src/conf/domain_conf.c | 155 +++++++++++-------
src/conf/domain_conf.h | 14 +-
src/conf/virconftypes.h | 2 +
src/qemu/qemu_capabilities.c | 2 +
src/qemu/qemu_capabilities.h | 1 +
src/qemu/qemu_cgroup.c | 4 +-
src/qemu/qemu_command.c | 70 +++++++-
src/qemu/qemu_driver.c | 3 +-
src/qemu/qemu_firmware.c | 34 ++--
src/qemu/qemu_namespace.c | 21 ++-
src/qemu/qemu_process.c | 34 +++-
src/qemu/qemu_validate.c | 31 +++-
src/security/security_dac.c | 6 +-
.../launch-security-s390-pv-ignore-policy.xml | 24 +++
.../launch-security-s390-pv.xml | 18 ++
.../launch-security-s390-pv-ignore-policy.xml | 1 +
tests/genericxml2xmltest.c | 2 +
.../qemucapabilitiesdata/caps_6.0.0.s390x.xml | 1 +
...ty-s390-pv-ignore-policy.s390x-latest.args | 35 ++++
.../launch-security-s390-pv-ignore-policy.xml | 33 ++++
.../launch-security-s390-pv.s390x-latest.args | 35 ++++
.../launch-security-s390-pv.xml | 30 ++++
...urity-sev-missing-policy.x86_64-2.12.0.err | 1 +
.../launch-security-sev-missing-policy.xml | 34 ++++
tests/qemuxml2argvtest.c | 4 +
28 files changed, 562 insertions(+), 108 deletions(-)
create mode 100644 tests/genericxml2xmlindata/launch-security-s390-pv-ignore-policy.xml
create mode 100644 tests/genericxml2xmlindata/launch-security-s390-pv.xml
create mode 120000 tests/genericxml2xmloutdata/launch-security-s390-pv-ignore-policy.xml
create mode 100644 tests/qemuxml2argvdata/launch-security-s390-pv-ignore-policy.s390x-latest.args
create mode 100644 tests/qemuxml2argvdata/launch-security-s390-pv-ignore-policy.xml
create mode 100644 tests/qemuxml2argvdata/launch-security-s390-pv.s390x-latest.args
create mode 100644 tests/qemuxml2argvdata/launch-security-s390-pv.xml
create mode 100644 tests/qemuxml2argvdata/launch-security-sev-missing-policy.x86_64-2.12.0.err
create mode 100644 tests/qemuxml2argvdata/launch-security-sev-missing-policy.xml
--
2.30.2
3 years, 3 months
[PATCH libvirt v1] conf: verify for duplicate hostdevs
by Shalini Chellathurai Saroja
It is possible to define/edit(in shut off state) a domain XML with
same hostdev device repeated more than once, as shown below. This
behavior is not expected. So, this patch fixes it.
vser1:
<domain type='kvm'>
[...]
<devices>
[...]
<hostdev mode='subsystem' type='mdev' managed='no' model='vfio-ccw'>
<source>
<address uuid='8e782fea-e5f4-45fa-a0f9-024cf66e5009'/>
</source>
<address type='ccw' cssid='0xfe' ssid='0x0' devno='0x0005'/>
</hostdev>
<hostdev mode='subsystem' type='mdev' managed='no' model='vfio-ccw'>
<source>
<address uuid='8e782fea-e5f4-45fa-a0f9-024cf66e5009'/>
</source>
<address type='ccw' cssid='0xfe' ssid='0x0' devno='0x0006'/>
</hostdev>
[...]
</devices>
</domain>
$ virsh define vser1
Domain 'vser1' defined from vser1
Signed-off-by: Shalini Chellathurai Saroja <shalini(a)linux.ibm.com>
Reviewed-by: Bjoern Walk <bwalk(a)linux.ibm.com>
Reviewed-by: Boris Fiuczynski <fiuczy(a)linux.ibm.com>
---
src/conf/domain_conf.c | 2 +-
src/conf/domain_conf.h | 2 +
src/conf/domain_validate.c | 21 ++++++++++
src/libvirt_private.syms | 1 +
.../hostdev-mdev-duplicate.err | 1 +
.../hostdev-mdev-duplicate.xml | 41 +++++++++++++++++++
.../hostdev-pci-duplicate.err | 1 +
.../hostdev-pci-duplicate.xml | 40 ++++++++++++++++++
.../hostdev-scsi-duplicate.err | 1 +
.../hostdev-scsi-duplicate.xml | 40 ++++++++++++++++++
.../hostdev-usb-duplicate.err | 1 +
.../hostdev-usb-duplicate.xml | 40 ++++++++++++++++++
tests/qemuxml2argvtest.c | 8 ++++
13 files changed, 198 insertions(+), 1 deletion(-)
create mode 100644 tests/qemuxml2argvdata/hostdev-mdev-duplicate.err
create mode 100644 tests/qemuxml2argvdata/hostdev-mdev-duplicate.xml
create mode 100644 tests/qemuxml2argvdata/hostdev-pci-duplicate.err
create mode 100644 tests/qemuxml2argvdata/hostdev-pci-duplicate.xml
create mode 100644 tests/qemuxml2argvdata/hostdev-scsi-duplicate.err
create mode 100644 tests/qemuxml2argvdata/hostdev-scsi-duplicate.xml
create mode 100644 tests/qemuxml2argvdata/hostdev-usb-duplicate.err
create mode 100644 tests/qemuxml2argvdata/hostdev-usb-duplicate.xml
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index f65509d8..5746f69e 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -15369,7 +15369,7 @@ virDomainHostdevMatchCaps(virDomainHostdevDef *a,
}
-static int
+int
virDomainHostdevMatch(virDomainHostdevDef *a,
virDomainHostdevDef *b)
{
diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h
index f706c498..4d9d499b 100644
--- a/src/conf/domain_conf.h
+++ b/src/conf/domain_conf.h
@@ -3590,6 +3590,8 @@ virDomainHostdevDef *
virDomainHostdevRemove(virDomainDef *def, size_t i);
int virDomainHostdevFind(virDomainDef *def, virDomainHostdevDef *match,
virDomainHostdevDef **found);
+int virDomainHostdevMatch(virDomainHostdevDef *a,
+ virDomainHostdevDef *b);
virDomainGraphicsListenDef *
virDomainGraphicsGetListen(virDomainGraphicsDef *def, size_t i);
diff --git a/src/conf/domain_validate.c b/src/conf/domain_validate.c
index 2124d25d..df2ab473 100644
--- a/src/conf/domain_validate.c
+++ b/src/conf/domain_validate.c
@@ -1068,7 +1068,25 @@ virDomainDefDuplicateDiskInfoValidate(const virDomainDef *def)
return 0;
}
+static int
+virDomainDefDuplicateHostdevInfoValidate(const virDomainDef *def)
+{
+ size_t i;
+ size_t j;
+ for (i = 0; i < def->nhostdevs; i++) {
+ for (j = i + 1; j < def->nhostdevs; j++) {
+ if (virDomainHostdevMatch(def->hostdevs[i],
+ def->hostdevs[j])) {
+ virReportError(VIR_ERR_XML_ERROR, "%s",
+ _("Hostdev already exists in the domain configuration"));
+ return -1;
+ }
+ }
+ }
+
+ return 0;
+}
/**
* virDomainDefDuplicateDriveAddressesValidate:
@@ -1529,6 +1547,9 @@ virDomainDefValidateInternal(const virDomainDef *def,
if (virDomainDefDuplicateDiskInfoValidate(def) < 0)
return -1;
+ if (virDomainDefDuplicateHostdevInfoValidate(def) < 0)
+ return -1;
+
if (virDomainDefDuplicateDriveAddressesValidate(def) < 0)
return -1;
diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
index 2efa7876..f0d1b7b4 100644
--- a/src/libvirt_private.syms
+++ b/src/libvirt_private.syms
@@ -457,6 +457,7 @@ virDomainHostdevDefFree;
virDomainHostdevDefNew;
virDomainHostdevFind;
virDomainHostdevInsert;
+virDomainHostdevMatch;
virDomainHostdevModeTypeToString;
virDomainHostdevRemove;
virDomainHostdevSubsysPCIBackendTypeToString;
diff --git a/tests/qemuxml2argvdata/hostdev-mdev-duplicate.err b/tests/qemuxml2argvdata/hostdev-mdev-duplicate.err
new file mode 100644
index 00000000..590afd30
--- /dev/null
+++ b/tests/qemuxml2argvdata/hostdev-mdev-duplicate.err
@@ -0,0 +1 @@
+XML error: Hostdev already exists in the domain configuration
diff --git a/tests/qemuxml2argvdata/hostdev-mdev-duplicate.xml b/tests/qemuxml2argvdata/hostdev-mdev-duplicate.xml
new file mode 100644
index 00000000..1c5e3263
--- /dev/null
+++ b/tests/qemuxml2argvdata/hostdev-mdev-duplicate.xml
@@ -0,0 +1,41 @@
+<domain type='qemu'>
+ <name>QEMUGuest2</name>
+ <uuid>c7a5fdbd-edaf-9455-926a-d65c16db1809</uuid>
+ <memory unit='KiB'>219136</memory>
+ <currentMemory unit='KiB'>219136</currentMemory>
+ <vcpu placement='static'>1</vcpu>
+ <os>
+ <type arch='i686' machine='pc'>hvm</type>
+ <boot dev='hd'/>
+ </os>
+ <clock offset='utc'/>
+ <on_poweroff>destroy</on_poweroff>
+ <on_reboot>restart</on_reboot>
+ <on_crash>destroy</on_crash>
+ <devices>
+ <emulator>/usr/bin/qemu-system-i386</emulator>
+ <controller type='usb' index='0'>
+ <address type='pci' domain='0x0000' bus='0x00' slot='0x01' function='0x2'/>
+ </controller>
+ <controller type='pci' index='0' model='pci-root'/>
+ <controller type='ide' index='0'>
+ <address type='pci' domain='0x0000' bus='0x00' slot='0x01' function='0x1'/>
+ </controller>
+ <input type='mouse' bus='ps2'/>
+ <input type='keyboard' bus='ps2'/>
+ <audio id='1' type='none'/>
+ <hostdev mode='subsystem' type='mdev' managed='no' model='vfio-pci'>
+ <source>
+ <address uuid='53764d0e-85a0-42b4-af5c-2046b460b1dc'/>
+ </source>
+ <address type='pci' domain='0x0000' bus='0x00' slot='0x02' function='0x0'/>
+ </hostdev>
+ <hostdev mode='subsystem' type='mdev' managed='no' model='vfio-pci'>
+ <source>
+ <address uuid='53764d0e-85a0-42b4-af5c-2046b460b1dc'/>
+ </source>
+ <address type='pci' domain='0x0000' bus='0x00' slot='0x02' function='0x1'/>
+ </hostdev>
+ <memballoon model='none'/>
+ </devices>
+</domain>
diff --git a/tests/qemuxml2argvdata/hostdev-pci-duplicate.err b/tests/qemuxml2argvdata/hostdev-pci-duplicate.err
new file mode 100644
index 00000000..590afd30
--- /dev/null
+++ b/tests/qemuxml2argvdata/hostdev-pci-duplicate.err
@@ -0,0 +1 @@
+XML error: Hostdev already exists in the domain configuration
diff --git a/tests/qemuxml2argvdata/hostdev-pci-duplicate.xml b/tests/qemuxml2argvdata/hostdev-pci-duplicate.xml
new file mode 100644
index 00000000..c744fddf
--- /dev/null
+++ b/tests/qemuxml2argvdata/hostdev-pci-duplicate.xml
@@ -0,0 +1,40 @@
+<domain type='qemu'>
+ <name>QEMUGuest2</name>
+ <uuid>c7a5fdbd-edaf-9466-926a-d65c16db1809</uuid>
+ <memory unit='KiB'>219100</memory>
+ <currentMemory unit='KiB'>219100</currentMemory>
+ <vcpu placement='static'>1</vcpu>
+ <os>
+ <type arch='i686' machine='pc'>hvm</type>
+ <boot dev='hd'/>
+ </os>
+ <clock offset='utc'/>
+ <on_poweroff>destroy</on_poweroff>
+ <on_reboot>restart</on_reboot>
+ <on_crash>destroy</on_crash>
+ <devices>
+ <emulator>/usr/bin/qemu-system-i386</emulator>
+ <disk type='block' device='disk'>
+ <driver name='qemu' type='raw'/>
+ <source dev='/dev/HostVG/QEMUGuest2'/>
+ <target dev='hda' bus='ide'/>
+ <address type='drive' controller='0' bus='0' target='0' unit='0'/>
+ </disk>
+ <controller type='usb' index='0'/>
+ <controller type='ide' index='0'/>
+ <controller type='pci' index='0' model='pci-root'/>
+ <input type='mouse' bus='ps2'/>
+ <input type='keyboard' bus='ps2'/>
+ <hostdev mode='subsystem' type='pci' managed='yes'>
+ <source>
+ <address domain='0x0000' bus='0x06' slot='0x12' function='0x5'/>
+ </source>
+ </hostdev>
+ <hostdev mode='subsystem' type='pci' managed='yes'>
+ <source>
+ <address domain='0x0000' bus='0x06' slot='0x12' function='0x5'/>
+ </source>
+ </hostdev>
+ <memballoon model='none'/>
+ </devices>
+</domain>
diff --git a/tests/qemuxml2argvdata/hostdev-scsi-duplicate.err b/tests/qemuxml2argvdata/hostdev-scsi-duplicate.err
new file mode 100644
index 00000000..590afd30
--- /dev/null
+++ b/tests/qemuxml2argvdata/hostdev-scsi-duplicate.err
@@ -0,0 +1 @@
+XML error: Hostdev already exists in the domain configuration
diff --git a/tests/qemuxml2argvdata/hostdev-scsi-duplicate.xml b/tests/qemuxml2argvdata/hostdev-scsi-duplicate.xml
new file mode 100644
index 00000000..ea367de3
--- /dev/null
+++ b/tests/qemuxml2argvdata/hostdev-scsi-duplicate.xml
@@ -0,0 +1,40 @@
+<domain type='qemu'>
+ <name>QEMUGuest2</name>
+ <uuid>c7a5fdbd-edaf-9466-926a-d65c16db1809</uuid>
+ <memory unit='KiB'>219100</memory>
+ <currentMemory unit='KiB'>219100</currentMemory>
+ <vcpu placement='static'>1</vcpu>
+ <os>
+ <type arch='i686' machine='pc'>hvm</type>
+ <boot dev='hd'/>
+ </os>
+ <clock offset='utc'/>
+ <on_poweroff>destroy</on_poweroff>
+ <on_reboot>restart</on_reboot>
+ <on_crash>destroy</on_crash>
+ <devices>
+ <emulator>/usr/bin/qemu-system-i386</emulator>
+ <disk type='block' device='disk'>
+ <source dev='/dev/HostVG/QEMUGuest2'/>
+ <target dev='hda' bus='ide'/>
+ <address type='drive' controller='0' bus='0' target='0' unit='0'/>
+ </disk>
+ <controller type='scsi' index='0' model='virtio-scsi'/>
+ <controller type='usb' index='0'/>
+ <controller type='ide' index='0'/>
+ <controller type='pci' index='0' model='pci-root'/>
+ <hostdev mode='subsystem' type='scsi' managed='yes'>
+ <source>
+ <adapter name='scsi_host0'/>
+ <address bus='0' target='0' unit='0'/>
+ </source>
+ </hostdev>
+ <hostdev mode='subsystem' type='scsi' managed='yes'>
+ <source>
+ <adapter name='scsi_host0'/>
+ <address bus='0' target='0' unit='0'/>
+ </source>
+ </hostdev>
+ <memballoon model='virtio'/>
+ </devices>
+</domain>
diff --git a/tests/qemuxml2argvdata/hostdev-usb-duplicate.err b/tests/qemuxml2argvdata/hostdev-usb-duplicate.err
new file mode 100644
index 00000000..590afd30
--- /dev/null
+++ b/tests/qemuxml2argvdata/hostdev-usb-duplicate.err
@@ -0,0 +1 @@
+XML error: Hostdev already exists in the domain configuration
diff --git a/tests/qemuxml2argvdata/hostdev-usb-duplicate.xml b/tests/qemuxml2argvdata/hostdev-usb-duplicate.xml
new file mode 100644
index 00000000..533a9059
--- /dev/null
+++ b/tests/qemuxml2argvdata/hostdev-usb-duplicate.xml
@@ -0,0 +1,40 @@
+<domain type='qemu'>
+ <name>QEMUGuest1</name>
+ <uuid>c7a5fdbd-edaf-9455-926a-d65c16db1809</uuid>
+ <memory unit='KiB'>219136</memory>
+ <currentMemory unit='KiB'>219136</currentMemory>
+ <vcpu placement='static'>1</vcpu>
+ <os>
+ <type arch='i686' machine='pc'>hvm</type>
+ <boot dev='hd'/>
+ </os>
+ <clock offset='utc'/>
+ <on_poweroff>destroy</on_poweroff>
+ <on_reboot>restart</on_reboot>
+ <on_crash>destroy</on_crash>
+ <devices>
+ <emulator>/usr/bin/qemu-system-i386</emulator>
+ <disk type='block' device='disk'>
+ <driver name='qemu' type='raw'/>
+ <source dev='/dev/HostVG/QEMUGuest1'/>
+ <target dev='hda' bus='ide'/>
+ <address type='drive' controller='0' bus='0' target='0' unit='0'/>
+ </disk>
+ <controller type='usb' index='0'/>
+ <controller type='ide' index='0'/>
+ <controller type='pci' index='0' model='pci-root'/>
+ <input type='mouse' bus='ps2'/>
+ <input type='keyboard' bus='ps2'/>
+ <hostdev mode='subsystem' type='usb' managed='no'>
+ <source>
+ <address bus='14' device='6'/>
+ </source>
+ </hostdev>
+ <hostdev mode='subsystem' type='usb' managed='no'>
+ <source>
+ <address bus='14' device='6'/>
+ </source>
+ </hostdev>
+ <memballoon model='none'/>
+ </devices>
+</domain>
diff --git a/tests/qemuxml2argvtest.c b/tests/qemuxml2argvtest.c
index 7fed871c..b9f96532 100644
--- a/tests/qemuxml2argvtest.c
+++ b/tests/qemuxml2argvtest.c
@@ -1915,8 +1915,11 @@ mymain(void)
DO_TEST("hostdev-usb-address", NONE);
DO_TEST("hostdev-usb-address-device", NONE);
DO_TEST("hostdev-usb-address-device-boot", NONE);
+ DO_TEST_PARSE_ERROR("hostdev-usb-duplicate", NONE);
DO_TEST("hostdev-pci-address", QEMU_CAPS_DEVICE_VFIO_PCI);
DO_TEST("hostdev-pci-address-device", QEMU_CAPS_DEVICE_VFIO_PCI);
+ DO_TEST_PARSE_ERROR("hostdev-pci-duplicate",
+ QEMU_CAPS_DEVICE_VFIO_PCI);
DO_TEST("hostdev-vfio",
QEMU_CAPS_DEVICE_VFIO_PCI);
DO_TEST("hostdev-vfio-multidomain",
@@ -1927,6 +1930,8 @@ mymain(void)
QEMU_CAPS_DEVICE_VFIO_PCI);
DO_TEST_PARSE_ERROR("hostdev-mdev-invalid-target-address",
QEMU_CAPS_DEVICE_VFIO_PCI);
+ DO_TEST_PARSE_ERROR("hostdev-mdev-duplicate",
+ QEMU_CAPS_DEVICE_VFIO_PCI);
DO_TEST_CAPS_LATEST("hostdev-mdev-display-spice-opengl");
DO_TEST_CAPS_LATEST("hostdev-mdev-display-spice-egl-headless");
DO_TEST_CAPS_LATEST("hostdev-mdev-display-vnc");
@@ -2856,6 +2861,9 @@ mymain(void)
QEMU_CAPS_VIRTIO_SCSI, QEMU_CAPS_DEVICE_VHOST_SCSI,
QEMU_CAPS_DEVICE_PCIE_ROOT_PORT,
QEMU_CAPS_VIRTIO_PCI_DISABLE_LEGACY);
+ DO_TEST_PARSE_ERROR("hostdev-scsi-duplicate",
+ QEMU_CAPS_VIRTIO_SCSI,
+ QEMU_CAPS_DEVICE_VHOST_SCSI);
DO_TEST_CAPS_VER("mlock-on", "3.0.0");
DO_TEST_CAPS_VER("mlock-off", "3.0.0");
--
2.30.2
3 years, 4 months
[libvirt PATCH v2 0/5] mdev tweaks
by Jonathon Jongsma
A few minor fixes to mdev support in the nodedev driver
Changes in v2:
- split out the error-reporting macro into a separate commit as recommended by Peter
- Since virCommandRun() may report an error, ensure that the
virMdevctl$COMMAND() functions always set an error to make error-handling
consistent. v1 tried to return an error message and have the caller report
the error.
- Added a new patch (destroying inactive device)
Jonathon Jongsma (5):
nodedev: Remove useless device name from error message
nodedev: Handle NULL command variable
nodedev: add macro to handle command errors
nodedev: handle mdevctl errors consistently
nodedev: improve error message when destroying an inactive device
src/node_device/node_device_driver.c | 147 +++++++++++++++++----------
1 file changed, 94 insertions(+), 53 deletions(-)
--
2.31.1
3 years, 4 months
[libvirt PATCH] nodedev: handle mdevs from multiple parents
by Jonathon Jongsma
Due to a rather unfortunate misunderstanding, we were parsing the list
of defined devices from mdevctl incorrectly. Since my primary
development machine only has a single device capable of mdevs, I
apparently neglected to test multiple parent devices and made some
assumptions based on reading the mdevctl code. These assumptions turned
out to be incorrect, so the parsing failed when devices from more than
one parent device were returned.
The details: mdevctl returns an array of objects representing the
defined devices. But instead of an array of multiple objects (with each
object representing a parent device), the array always contains only a
single object. That object has a separate property for each parent
device.
Signed-off-by: Jonathon Jongsma <jjongsma(a)redhat.com>
---
src/node_device/node_device_driver.c | 41 ++++++++++---------
.../mdevctl-list-multiple.json | 4 +-
2 files changed, 23 insertions(+), 22 deletions(-)
diff --git a/src/node_device/node_device_driver.c b/src/node_device/node_device_driver.c
index 8a0a2c3847..cb2c3ceaa4 100644
--- a/src/node_device/node_device_driver.c
+++ b/src/node_device/node_device_driver.c
@@ -1056,6 +1056,7 @@ nodeDeviceParseMdevctlJSON(const char *jsonstring,
size_t noutdevs = 0;
size_t i;
size_t j;
+ virJSONValue *obj;
json_devicelist = virJSONValueFromString(jsonstring);
@@ -1065,31 +1066,33 @@ nodeDeviceParseMdevctlJSON(const char *jsonstring,
goto error;
}
- n = virJSONValueArraySize(json_devicelist);
+ /* mdevctl list --dumpjson produces an output that is an array that
+ * contains only a single object which contains a property for each parent
+ * device */
+ if (virJSONValueArraySize(json_devicelist) != 1) {
+ virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
+ _("Unexpected format for mdevctl response"));
+ goto error;
+ }
+
+ obj = virJSONValueArrayGet(json_devicelist, 0);
+
+ if (!virJSONValueIsObject(obj)) {
+ virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
+ _("device list is not an object"));
+ goto error;
+ }
+ n = virJSONValueObjectKeysNumber(obj);
for (i = 0; i < n; i++) {
- virJSONValue *obj = virJSONValueArrayGet(json_devicelist, i);
const char *parent;
virJSONValue *child_array;
int nchildren;
- if (!virJSONValueIsObject(obj)) {
- virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
- _("Parent device is not an object"));
- goto error;
- }
-
- /* mdevctl returns an array of objects. Each object is a parent device
- * object containing a single key-value pair which maps from the name
- * of the parent device to an array of child devices */
- if (virJSONValueObjectKeysNumber(obj) != 1) {
- virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
- _("Unexpected format for parent device object"));
- goto error;
- }
-
- parent = virJSONValueObjectGetKey(obj, 0);
- child_array = virJSONValueObjectGetValue(obj, 0);
+ /* The key of each object property is the name of a parent device
+ * which maps to an array of child devices */
+ parent = virJSONValueObjectGetKey(obj, i);
+ child_array = virJSONValueObjectGetValue(obj, i);
if (!virJSONValueIsArray(child_array)) {
virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
diff --git a/tests/nodedevmdevctldata/mdevctl-list-multiple.json b/tests/nodedevmdevctldata/mdevctl-list-multiple.json
index eefcd90c62..ca1918d00a 100644
--- a/tests/nodedevmdevctldata/mdevctl-list-multiple.json
+++ b/tests/nodedevmdevctldata/mdevctl-list-multiple.json
@@ -24,9 +24,7 @@
]
}
}
- ]
- },
- {
+ ],
"matrix": [
{ "783e6dbb-ea0e-411f-94e2-717eaad438bf": {
"mdev_type": "vfio_ap-passthrough",
--
2.31.1
3 years, 4 months
Avoiding lockspace already exists error from virtlockd
by Jim Fehlig
Hi All!
I received a bug report about virtlockd emitting an error whenever libvirtd is
(re)started
May 25 15:44:31 virt81 virtlockd[7723]: Requested operation is not valid:
Lockspace for path /data/libvirtd/lockspace already exists
The problem is easily reproducible with git master by enabling lockd in
qemu.conf, setting file_lockspace_dir in qemu-lockd.conf, then restarting libvirtd.
If I understand the code correctly, when the qemu driver loads, it calls
virLockManagerPluginNew, which dlopens lockd.so and calls drvInit, aka
virLockManagerLockDaemonInit. Here the driver object is created, config loaded,
and virLockManagerLockDaemonSetupLockspace is called.
virLockManagerLockDaemonSetupLockspace sends
virLockSpaceProtocolCreateLockSpaceArgs rpc to virtlockd, where it is dispatched
to virLockSpaceProtocolDispatchCreateLockSpace. Alas we encounter the error when
virLockDaemonFindLockSpace finds the existing lockspace.
I'm not really sure how to go about fixing it and fishing for opinions.
virLockManagerLockDaemonSetupLockspace already has some code to handle the error
https://gitlab.com/libvirt/libvirt/-/blob/master/src/locking/lock_driver_...
Since libvirtd ignores VIR_ERR_OPERATION_INVALID, should virtlockd be changed to
not return error in that case? It would be better if libvirtd knew it already
told virtlockd to configure the lockspace and avoid needlessly doing it again.
BTW, this cosmetic problem is exasperated by the default '--timeout 120' option
to libvirtd, since it results in more daemon restarts.
Thanks for any suggestions!
Jim
3 years, 4 months
[libvirt PATCH v2] security: fix SELinux label generation logic
by Daniel P. Berrangé
A process can access a file if the set of MCS categories
for the file is equal-to *or* a subset-of, the set of
MCS categories for the process.
If there are two VMs:
a) svirt_t:s0:c117
b) svirt_t:s0:c117,c720
Then VM (b) is able to access files labelled for VM (a).
IOW, we must discard case where the categories are equal
because that is a subset of many other valid category pairs.
Fixes: https://gitlab.com/libvirt/libvirt/-/issues/153
CVE-2021-3631
Reviewed-by: Peter Krempa <pkrempa(a)redhat.com>
Signed-off-by: Daniel P. Berrangé <berrange(a)redhat.com>
---
Pushed to git, just publishing v2 to show the CVE allocation
for the historical record.
src/security/security_selinux.c | 10 +++++++++-
1 file changed, 9 insertions(+), 1 deletion(-)
diff --git a/src/security/security_selinux.c b/src/security/security_selinux.c
index b50f4463cc..0c2cf1d1c7 100644
--- a/src/security/security_selinux.c
+++ b/src/security/security_selinux.c
@@ -383,7 +383,15 @@ virSecuritySELinuxMCSFind(virSecurityManager *mgr,
VIR_DEBUG("Try cat %s:c%d,c%d", sens, c1 + catMin, c2 + catMin);
if (c1 == c2) {
- mcs = g_strdup_printf("%s:c%d", sens, catMin + c1);
+ /*
+ * A process can access a file if the set of MCS categories
+ * for the file is equal-to *or* a subset-of, the set of
+ * MCS categories for the process.
+ *
+ * IOW, we must discard case where the categories are equal
+ * because that is a subset of other category pairs.
+ */
+ continue;
} else {
if (c1 > c2) {
int t = c1;
--
2.31.1
3 years, 4 months
[libvirt PATCH] security: fix SELinux label generation logic
by Daniel P. Berrangé
A process can access a file if the set of MCS categories
for the file is equal-to *or* a subset-of, the set of
MCS categories for the process.
If there are two VMs:
a) svirt_t:s0:c117
b) svirt_t:s0:c117,c720
Then VM (b) is able to access files labelled for VM (a).
IOW, we must discard case where the categories are equal
because that is a subset of many other valid category pairs.
Fixes: https://gitlab.com/libvirt/libvirt/-/issues/153
CVE-2021-xxxx - tbd before pushing
Signed-off-by: Daniel P. Berrangé <berrange(a)redhat.com>
---
src/security/security_selinux.c | 10 +++++++++-
1 file changed, 9 insertions(+), 1 deletion(-)
diff --git a/src/security/security_selinux.c b/src/security/security_selinux.c
index b50f4463cc..c98dab0d6f 100644
--- a/src/security/security_selinux.c
+++ b/src/security/security_selinux.c
@@ -383,7 +383,15 @@ virSecuritySELinuxMCSFind(virSecurityManager *mgr,
VIR_DEBUG("Try cat %s:c%d,c%d", sens, c1 + catMin, c2 + catMin);
if (c1 == c2) {
- mcs = g_strdup_printf("%s:c%d", sens, catMin + c1);
+ /*
+ * A process can access a file if the set of MCS categories
+ * for the file is equal-to *or* a subset-of, the set of
+ * MCS categories for the process.
+ *
+ * IOW, we must discard case where the categories are equal
+ * because that is a subset of other category pairs.
+ */
+ continue
} else {
if (c1 > c2) {
int t = c1;
--
2.31.1
3 years, 4 months