Re: [PATCH v3 5/9] domain_conf.c: XML parsing for VIR_DOMAIN_TPM_MODEL_SPAPR_PROXY
On 5/14/20 10:07 PM, Daniel Henrique Barboza wrote: On 5/14/20 11:32 AM, Daniel Henrique Barboza wrote: On 5/14/20 11:09 AM, Ján Tomko wrote: On a Wednesday in 2020, Daniel Henrique Barboza wrote: Aside from trivial XML parsing/format changes, this patch adds additional rules for TPM device support to better accomodate [...] Any reason why you store them separately? It seems they are treated the same in every place except when building QEMU command line. Switching to a def->tpms array would better reflect the XML. The Validate function would then check wheteher there's just one copy of each device type. Just sent a v4 with this approach. I attempted to do the trick I mentioned previously of having def->tpms[0] to always hold a non-proxy device, but in the end I decided it wasn't worth it - I would make everyone reading qemu_extdevices.c to wonder "why is this code passing def->tpms[0] instead of interacting the whole array". It was too messy for someone who doesn't know about the TPM Proxy history. Thanks, DHB
[PATCH v4 09/10] tests/qemuxml2argvtest.c: add TPM Proxy command line tests
Add tests for both supported scenarios: a single TPM Proxy and
a TPM Proxy with a regular TPM device in the same domain.
Reviewed-by: Stefan Berger
Signed-off-by: Daniel Henrique Barboza
---
.../ppc64-tpmproxy-single.ppc64-latest.args | 34 +
.../ppc64-tpmproxy-with-tpm.ppc64-latest.args | 37 +++
tests/qemuxml2argvtest.c | 3 ++
3 files changed, 74 insertions(+)
create mode 100644
tests/qemuxml2argvdata/ppc64-tpmproxy-single.ppc64-latest.args
create mode 100644
tests/qemuxml2argvdata/ppc64-tpmproxy-with-tpm.ppc64-latest.args
diff --git a/tests/qemuxml2argvdata/ppc64-tpmproxy-single.ppc64-latest.args
b/tests/qemuxml2argvdata/ppc64-tpmproxy-single.ppc64-latest.args
new file mode 100644
index 00..f606cee16b
--- /dev/null
+++ b/tests/qemuxml2argvdata/ppc64-tpmproxy-single.ppc64-latest.args
@@ -0,0 +1,34 @@
+LC_ALL=C \
+PATH=/bin \
+HOME=/tmp/lib/domain--1-QEMUGuest1 \
+USER=test \
+LOGNAME=test \
+XDG_DATA_HOME=/tmp/lib/domain--1-QEMUGuest1/.local/share \
+XDG_CACHE_HOME=/tmp/lib/domain--1-QEMUGuest1/.cache \
+XDG_CONFIG_HOME=/tmp/lib/domain--1-QEMUGuest1/.config \
+QEMU_AUDIO_DRV=none \
+/usr/bin/qemu-system-ppc64 \
+-name guest=QEMUGuest1,debug-threads=on \
+-S \
+-object secret,id=masterKey0,format=raw,\
+file=/tmp/lib/domain--1-QEMUGuest1/master-key.aes \
+-machine pseries,accel=tcg,usb=off,dump-guest-core=off \
+-cpu POWER9 \
+-m 256 \
+-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,nowait \
+-mon chardev=charmonitor,id=monitor,mode=control \
+-rtc base=utc \
+-no-shutdown \
+-boot strict=on \
+-device pci-ohci,id=usb,bus=pci.0,addr=0x1 \
+-device spapr-tpm-proxy,id=tpmproxy0,host-path=/dev/tpmrm0 \
+-device virtio-balloon-pci,id=balloon0,bus=pci.0,addr=0x6 \
+-sandbox on,obsolete=deny,elevateprivileges=deny,spawn=deny,\
+resourcecontrol=deny \
+-msg timestamp=on
diff --git a/tests/qemuxml2argvdata/ppc64-tpmproxy-with-tpm.ppc64-latest.args
b/tests/qemuxml2argvdata/ppc64-tpmproxy-with-tpm.ppc64-latest.args
new file mode 100644
index 00..83eb58ae19
--- /dev/null
+++ b/tests/qemuxml2argvdata/ppc64-tpmproxy-with-tpm.ppc64-latest.args
@@ -0,0 +1,37 @@
+LC_ALL=C \
+PATH=/bin \
+HOME=/tmp/lib/domain--1-QEMUGuest1 \
+USER=test \
+LOGNAME=test \
+XDG_DATA_HOME=/tmp/lib/domain--1-QEMUGuest1/.local/share \
+XDG_CACHE_HOME=/tmp/lib/domain--1-QEMUGuest1/.cache \
+XDG_CONFIG_HOME=/tmp/lib/domain--1-QEMUGuest1/.config \
+QEMU_AUDIO_DRV=none \
+/usr/bin/qemu-system-ppc64 \
+-name guest=QEMUGuest1,debug-threads=on \
+-S \
+-object secret,id=masterKey0,format=raw,\
+file=/tmp/lib/domain--1-QEMUGuest1/master-key.aes \
+-machine pseries,accel=tcg,usb=off,dump-guest-core=off \
+-cpu POWER9 \
+-m 256 \
+-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,nowait \
+-mon chardev=charmonitor,id=monitor,mode=control \
+-rtc base=utc \
+-no-shutdown \
+-boot strict=on \
+-device pci-ohci,id=usb,bus=pci.0,addr=0x1 \
+-device spapr-tpm-proxy,id=tpmproxy0,host-path=/dev/tpmrm0 \
+-tpmdev emulator,id=tpm-tpm0,chardev=chrtpm \
+-chardev socket,id=chrtpm,path=/dev/test \
+-device tpm-spapr,tpmdev=tpm-tpm0,id=tpm0,reg=0x4000 \
+-device virtio-balloon-pci,id=balloon0,bus=pci.0,addr=0x6 \
+-sandbox on,obsolete=deny,elevateprivileges=deny,spawn=deny,\
+resourcecontrol=deny \
+-msg timestamp=on
diff --git a/tests/qemuxml2argvtest.c b/tests/qemuxml2argvtest.c
index 1ce5e57d84..55a599c085 100644
--- a/tests/qemuxml2argvtest.c
+++ b/tests/qemuxml2argvtest.c
@@ -2974,6 +2974,9 @@ mymain(void)
QEMU_CAPS_DEVICE_TPM_PASSTHROUGH,
QEMU_CAPS_DEVICE_SPAPR_TPM_PROXY);
+DO_TEST_CAPS_LATEST_PPC64("ppc64-tpmproxy-single");
+DO_TEST_CAPS_LATEST_PPC64("ppc64-tpmproxy-with-tpm");
+
DO_TEST("aarch64-usb-controller-qemu-xhci",
QEMU_CAPS_OBJECT_GPEX,
QEMU_CAPS_NEC_USB_XHCI,
--
2.26.2
[PATCH v4 04/10] qemu_tpm, security, tests: change 'switch' clauses for 'if'
This trivial rework is aimed to reduce the amount of line changes
made by the next patch, when 'def->tpm' will become a 'def->tpms'
array.
Instead of using a 'switch' where only the VIR_DOMAIN_TPM_TYPE_EMULATOR
label does something, use an 'if' clause instead.
Signed-off-by: Daniel Henrique Barboza
---
src/qemu/qemu_tpm.c | 44 +
src/security/security_selinux.c | 16 ++--
src/security/virt-aa-helper.c | 7 +-
tests/qemuxml2argvtest.c| 7 +-
4 files changed, 10 insertions(+), 64 deletions(-)
diff --git a/src/qemu/qemu_tpm.c b/src/qemu/qemu_tpm.c
index fe567f440c..afec0e5328 100644
--- a/src/qemu/qemu_tpm.c
+++ b/src/qemu/qemu_tpm.c
@@ -680,14 +680,9 @@ qemuExtTPMInitPaths(virQEMUDriverPtr driver,
{
g_autoptr(virQEMUDriverConfig) cfg = virQEMUDriverGetConfig(driver);
-switch (def->tpm->type) {
-case VIR_DOMAIN_TPM_TYPE_EMULATOR:
+if (def->tpm->type == VIR_DOMAIN_TPM_TYPE_EMULATOR)
return qemuTPMEmulatorInitPaths(def->tpm, cfg->swtpmStorageDir,
def->uuid);
-case VIR_DOMAIN_TPM_TYPE_PASSTHROUGH:
-case VIR_DOMAIN_TPM_TYPE_LAST:
-break;
-}
return 0;
}
@@ -700,8 +695,7 @@ qemuExtTPMPrepareHost(virQEMUDriverPtr driver,
g_autoptr(virQEMUDriverConfig) cfg = virQEMUDriverGetConfig(driver);
g_autofree char *shortName = NULL;
-switch (def->tpm->type) {
-case VIR_DOMAIN_TPM_TYPE_EMULATOR:
+if (def->tpm->type == VIR_DOMAIN_TPM_TYPE_EMULATOR) {
shortName = virDomainDefGetShortName(def);
if (!shortName)
return -1;
@@ -711,9 +705,6 @@ qemuExtTPMPrepareHost(virQEMUDriverPtr driver,
cfg->swtpm_group,
cfg->swtpmStateDir, cfg->user,
shortName);
-case VIR_DOMAIN_TPM_TYPE_PASSTHROUGH:
-case VIR_DOMAIN_TPM_TYPE_LAST:
-break;
}
return 0;
@@ -723,15 +714,8 @@ qemuExtTPMPrepareHost(virQEMUDriverPtr driver,
void
qemuExtTPMCleanupHost(virDomainDefPtr def)
{
-switch (def->tpm->type) {
-case VIR_DOMAIN_TPM_TYPE_EMULATOR:
+if (def->tpm->type == VIR_DOMAIN_TPM_TYPE_EMULATOR)
qemuTPMDeleteEmulatorStorage(def->tpm);
-break;
-case VIR_DOMAIN_TPM_TYPE_PASSTHROUGH:
-case VIR_DOMAIN_TPM_TYPE_LAST:
-/* nothing to do */
-break;
-}
}
@@ -825,15 +809,9 @@ qemuExtTPMStart(virQEMUDriverPtr driver,
{
virDomainTPMDefPtr tpm = vm->def->tpm;
-switch (tpm->type) {
-case VIR_DOMAIN_TPM_TYPE_EMULATOR:
+if (tpm->type == VIR_DOMAIN_TPM_TYPE_EMULATOR)
return qemuExtTPMStartEmulator(driver, vm, incomingMigration);
-case VIR_DOMAIN_TPM_TYPE_PASSTHROUGH:
-case VIR_DOMAIN_TPM_TYPE_LAST:
-break;
-}
-
return 0;
}
@@ -845,18 +823,13 @@ qemuExtTPMStop(virQEMUDriverPtr driver,
g_autoptr(virQEMUDriverConfig) cfg = virQEMUDriverGetConfig(driver);
g_autofree char *shortName = NULL;
-switch (vm->def->tpm->type) {
-case VIR_DOMAIN_TPM_TYPE_EMULATOR:
+if (vm->def->tpm->type == VIR_DOMAIN_TPM_TYPE_EMULATOR) {
shortName = virDomainDefGetShortName(vm->def);
if (!shortName)
return;
qemuTPMEmulatorStop(cfg->swtpmStateDir, shortName);
qemuSecurityCleanupTPMEmulator(driver, vm);
-break;
-case VIR_DOMAIN_TPM_TYPE_PASSTHROUGH:
-case VIR_DOMAIN_TPM_TYPE_LAST:
-break;
}
return;
@@ -873,8 +846,7 @@ qemuExtTPMSetupCgroup(virQEMUDriverPtr driver,
int rc;
pid_t pid;
-switch (def->tpm->type) {
-case VIR_DOMAIN_TPM_TYPE_EMULATOR:
+if (def->tpm->type == VIR_DOMAIN_TPM_TYPE_EMULATOR) {
shortName = virDomainDefGetShortName(def);
if (!shortName)
return -1;
@@ -886,10 +858,6 @@ qemuExtTPMSetupCgroup(virQEMUDriverPtr driver,
}
if (virCgroupAddProcess(cgroup, pid) < 0)
return -1;
-break;
-case VIR_DOMAIN_TPM_TYPE_PASSTHROUGH:
-case VIR_DOMAIN_TPM_TYPE_LAST:
-break;
}
return 0;
diff --git a/src/security/security_selinux.c b/src/security/security_selinux.c
index 9a929debe1..914a252df1 100644
--- a/src/security/security_selinux.c
+++ b/src/security/security_selinux.c
@@ -3493,10 +3493,7 @@ virSecuritySELinuxSetTPMLabels(virSecurityManagerPtr mgr,
if (seclabel == NULL)
return 0;
-switch (def->tpm->type) {
-case VIR_DOMAIN_TPM_TYPE_PASSTHROUGH:
-break;
-case VIR_DOMAIN_TPM_TYPE_EMULATOR:
+if (def->tpm->type == VIR_DOMAIN_TPM_TYPE_EMULATOR) {
ret = virSecuritySELinuxSetFileLabels(
mgr, def->tpm->data.emulator.storagepath,
seclabel);
@@ -3504,9 +3501,6 @@ virSecuritySELinuxSetTPMLabels(virSecurityManagerPtr mgr,
ret = virSecuritySELinuxSetFileLabels(
mgr,
[PATCH v4 10/10] docs/news.xml: update for the new TPM Proxy device
Reviewed-by: Stefan Berger Signed-off-by: Daniel Henrique Barboza --- docs/news.xml | 17 + 1 file changed, 17 insertions(+) diff --git a/docs/news.xml b/docs/news.xml index 4cef804aac..c22a0f0a18 100644 --- a/docs/news.xml +++ b/docs/news.xml @@ -44,6 +44,23 @@ + + + qemu: add TPM Proxy device support + + + libvirt can now create guests using a new device type called + "TPM Proxy". The TPM Proxy connects to a TPM Resource Manager + present in the host, enabling the guest to run in secure virtual + machine mode with the help of an Ultravisor. Adding a TPM Proxy to + a pSeries guest brings no security benefits unless the guest is + running on a PPC64 host that has Ultravisor and TPM Resource Manager + support. Only one TPM Proxy is allowed per guest. A guest using + a TPM Proxy device can instantiate another TPM device at the same + time. This device is supported only for pSeries guests via the new + 'spapr-tpm-proxy' model of the TPM 'passthrough' backend. + + -- 2.26.2
[PATCH v4 07/10] tests: add XML schema tests for the TPM Proxy device
This tests aims to exercise how a TPM Proxy device can be
added in the domain, either alone or with a regular TPM
device. It also ensures that we do not allow bogus scenarios
to slip by.
Reviewed-by: Stefan Berger
Signed-off-by: Daniel Henrique Barboza
---
tests/qemuxml2argvdata/ppc64-tpm-double.xml | 34 ++
.../ppc64-tpmproxy-double.xml | 38 +++
.../ppc64-tpmproxy-single.xml | 33 +
.../ppc64-tpmproxy-with-tpm.xml | 36 +++
tests/qemuxml2argvtest.c | 12 +
.../ppc64-tpmproxy-single.ppc64-latest.xml| 42 +
.../ppc64-tpmproxy-with-tpm.ppc64-latest.xml | 46 +++
tests/qemuxml2xmltest.c | 2 +
8 files changed, 243 insertions(+)
create mode 100644 tests/qemuxml2argvdata/ppc64-tpm-double.xml
create mode 100644 tests/qemuxml2argvdata/ppc64-tpmproxy-double.xml
create mode 100644 tests/qemuxml2argvdata/ppc64-tpmproxy-single.xml
create mode 100644 tests/qemuxml2argvdata/ppc64-tpmproxy-with-tpm.xml
create mode 100644
tests/qemuxml2xmloutdata/ppc64-tpmproxy-single.ppc64-latest.xml
create mode 100644
tests/qemuxml2xmloutdata/ppc64-tpmproxy-with-tpm.ppc64-latest.xml
diff --git a/tests/qemuxml2argvdata/ppc64-tpm-double.xml
b/tests/qemuxml2argvdata/ppc64-tpm-double.xml
new file mode 100644
index 00..8730547a4d
--- /dev/null
+++ b/tests/qemuxml2argvdata/ppc64-tpm-double.xml
@@ -0,0 +1,34 @@
+
+ QEMUGuest1
+ c7a5fdbd-edaf-9455-926a-d65c16db1809
+ 219100
+ 219100
+ 1
+
+hvm
+
+
+
+
+
+
+
+ destroy
+ restart
+ restart
+
+/usr/bin/qemu-system-ppc64
+
+
+
+
+
+
+
+
+
+
+
+
+
+
diff --git a/tests/qemuxml2argvdata/ppc64-tpmproxy-double.xml
b/tests/qemuxml2argvdata/ppc64-tpmproxy-double.xml
new file mode 100644
index 00..12abda509e
--- /dev/null
+++ b/tests/qemuxml2argvdata/ppc64-tpmproxy-double.xml
@@ -0,0 +1,38 @@
+
+ QEMUGuest1
+ c7a5fdbd-edaf-9455-926a-d65c16db1809
+ 219100
+ 219100
+ 1
+
+hvm
+
+
+
+
+
+
+
+ destroy
+ restart
+ restart
+
+/usr/bin/qemu-system-ppc64
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
diff --git a/tests/qemuxml2argvdata/ppc64-tpmproxy-single.xml
b/tests/qemuxml2argvdata/ppc64-tpmproxy-single.xml
new file mode 100644
index 00..729a2cdf28
--- /dev/null
+++ b/tests/qemuxml2argvdata/ppc64-tpmproxy-single.xml
@@ -0,0 +1,33 @@
+
+ QEMUGuest1
+ c7a5fdbd-edaf-9455-926a-d65c16db1809
+ 219100
+ 219100
+ 1
+
+hvm
+
+
+
+
+
+
+
+ destroy
+ restart
+ restart
+
+/usr/bin/qemu-system-ppc64
+
+
+
+
+
+
+
+
+
+
+
+
+
diff --git a/tests/qemuxml2argvdata/ppc64-tpmproxy-with-tpm.xml
b/tests/qemuxml2argvdata/ppc64-tpmproxy-with-tpm.xml
new file mode 100644
index 00..a61ec9845c
--- /dev/null
+++ b/tests/qemuxml2argvdata/ppc64-tpmproxy-with-tpm.xml
@@ -0,0 +1,36 @@
+
+ QEMUGuest1
+ c7a5fdbd-edaf-9455-926a-d65c16db1809
+ 219100
+ 219100
+ 1
+
+hvm
+
+
+
+
+
+
+
+ destroy
+ restart
+ restart
+
+/usr/bin/qemu-system-ppc64
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
diff --git a/tests/qemuxml2argvtest.c b/tests/qemuxml2argvtest.c
index e5f129ea4c..1ce5e57d84 100644
--- a/tests/qemuxml2argvtest.c
+++ b/tests/qemuxml2argvtest.c
@@ -2962,6 +2962,18 @@ mymain(void)
QEMU_CAPS_NEC_USB_XHCI,
QEMU_CAPS_DEVICE_QEMU_XHCI);
+DO_TEST_PARSE_ERROR("ppc64-tpmproxy-double",
+QEMU_CAPS_DEVICE_SPAPR_PCI_HOST_BRIDGE,
+QEMU_CAPS_PCI_OHCI,
+QEMU_CAPS_DEVICE_TPM_PASSTHROUGH,
+QEMU_CAPS_DEVICE_SPAPR_TPM_PROXY);
+
+DO_TEST_PARSE_ERROR("ppc64-tpm-double",
+QEMU_CAPS_DEVICE_SPAPR_PCI_HOST_BRIDGE,
+QEMU_CAPS_PCI_OHCI,
+QEMU_CAPS_DEVICE_TPM_PASSTHROUGH,
+QEMU_CAPS_DEVICE_SPAPR_TPM_PROXY);
+
DO_TEST("aarch64-usb-controller-qemu-xhci",
QEMU_CAPS_OBJECT_GPEX,
QEMU_CAPS_NEC_USB_XHCI,
diff --git a/tests/qemuxml2xmloutdata/ppc64-tpmproxy-single.ppc64-latest.xml
b/tests/qemuxml2xmloutdata/ppc64-tpmproxy-single.ppc64-latest.xml
new file mode 100644
index 00..4e0e5f24b8
--- /dev/null
+++ b/tests/qemuxml2xmloutdata/ppc64-tpmproxy-single.ppc64-latest.xml
@@ -0,0 +1,42 @@
+
+ QEMUGuest1
+ c7a5fdbd-edaf-9455-926a-d65c16db1809
+ 219100
+ 219100
+ 1
+
+hvm
+
+
+
+
+
+
+
+
+POWER9
+
+
+ destroy
+ restart
+ restart
+
+/usr/bin/qemu-system-ppc64
+
+
+
+
+
+
+
+
+
+
[PATCH v4 05/10] conf, qemu, security, tests: introducing 'def->tpms' array
A TPM Proxy device can coexist with a regular TPM, but the
current domain definition supports only a single TPM device
in the 'tpm' pointer. This patch replaces this existing pointer
in the domain definition to an array of TPM devices.
All files that references the old pointer were adapted to
handle the new array instead. virDomainDefParseXML() TPM related
code was adapted to guarantee that the following combinations in the
same domain are valid:
- a single TPM device
- a single TPM Proxy device
- a single TPM + single TPM Proxy devices
And, these combinations in the same domain are NOT valid:
- 2 or more TPM devices
- 2 or more TPM Proxy devices
Signed-off-by: Daniel Henrique Barboza
---
src/conf/domain_audit.c | 4 +-
src/conf/domain_conf.c | 72 -
src/conf/domain_conf.h | 6 ++-
src/qemu/qemu_alias.c | 4 +-
src/qemu/qemu_cgroup.c | 10 +++--
src/qemu/qemu_command.c | 34 +++-
src/qemu/qemu_domain.c | 31 +-
src/qemu/qemu_domain_address.c | 11 +++--
src/qemu/qemu_extdevice.c | 18 +
src/qemu/qemu_tpm.c | 52 ++--
src/security/security_dac.c | 8 ++--
src/security/security_selinux.c | 32 +--
src/security/virt-aa-helper.c | 9 +++--
tests/qemuxml2argvtest.c| 13 +++---
14 files changed, 208 insertions(+), 96 deletions(-)
diff --git a/src/conf/domain_audit.c b/src/conf/domain_audit.c
index 1b0abb21a0..8bc6633af4 100644
--- a/src/conf/domain_audit.c
+++ b/src/conf/domain_audit.c
@@ -821,8 +821,8 @@ virDomainAuditStart(virDomainObjPtr vm, const char *reason,
bool success)
for (i = 0; i < vm->def->nrngs; i++)
virDomainAuditRNG(vm, NULL, vm->def->rngs[i], "start", true);
-if (vm->def->tpm)
-virDomainAuditTPM(vm, vm->def->tpm, "start", true);
+for (i = 0; i < vm->def->ntpms; i++)
+virDomainAuditTPM(vm, vm->def->tpms[i], "start", true);
for (i = 0; i < vm->def->nshmems; i++)
virDomainAuditShmem(vm, vm->def->shmems[i], "start", true);
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index c201fc901d..f8fee62c0c 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -1165,6 +1165,7 @@ VIR_ENUM_IMPL(virDomainTPMModel,
"tpm-tis",
"tpm-crb",
"tpm-spapr",
+ "spapr-tpm-proxy",
);
VIR_ENUM_IMPL(virDomainTPMBackend,
@@ -3479,7 +3480,9 @@ void virDomainDefFree(virDomainDefPtr def)
virDomainMemoryDefFree(def->mems[i]);
VIR_FREE(def->mems);
-virDomainTPMDefFree(def->tpm);
+for (i = 0; i < def->ntpms; i++)
+virDomainTPMDefFree(def->tpms[i]);
+VIR_FREE(def->tpms);
for (i = 0; i < def->npanics; i++)
virDomainPanicDefFree(def->panics[i]);
@@ -4312,10 +4315,10 @@ virDomainDeviceInfoIterateInternal(virDomainDefPtr def,
if ((rc = cb(def, , >shmems[i]->info, opaque)) != 0)
return rc;
}
-if (def->tpm) {
-device.type = VIR_DOMAIN_DEVICE_TPM;
-device.data.tpm = def->tpm;
-if ((rc = cb(def, , >tpm->info, opaque)) != 0)
+device.type = VIR_DOMAIN_DEVICE_TPM;
+for (i = 0; i < def->ntpms; i++) {
+device.data.tpm = def->tpms[i];
+if ((rc = cb(def, , >tpms[i]->info, opaque)) != 0)
return rc;
}
device.type = VIR_DOMAIN_DEVICE_PANIC;
@@ -21964,15 +21967,45 @@ virDomainDefParseXML(xmlDocPtr xml,
if ((n = virXPathNodeSet("./devices/tpm", ctxt, )) < 0)
goto error;
-if (n > 1) {
+if (n > 2) {
virReportError(VIR_ERR_XML_ERROR, "%s",
- _("only a single TPM device is supported"));
+ _("a maximum of two TPM devices is supported, one of "
+ "them being a TPM Proxy device"));
goto error;
}
+if (n && VIR_ALLOC_N(def->tpms, n) < 0)
+goto error;
+
if (n > 0) {
-if (!(def->tpm = virDomainTPMDefParseXML(xmlopt, nodes[0], ctxt,
flags)))
-goto error;
+virDomainTPMDefPtr proxyTPM = NULL;
+virDomainTPMDefPtr regularTPM = NULL;
+
+for (i = 0; i < n; i++) {
+virDomainTPMDefPtr tpm = virDomainTPMDefParseXML(xmlopt, nodes[i],
+ ctxt, flags);
+
+if (!tpm)
+goto error;
+
+if (tpm->model == VIR_DOMAIN_TPM_MODEL_SPAPR_PROXY) {
+if (proxyTPM) {
+virReportError(VIR_ERR_XML_ERROR, "%s",
+ _("only a single TPM Proxy device is
supported"));
+goto error;
+} else {
+proxyTPM = tpm;
+}
+} else if (regularTPM) {
+virReportError(VIR_ERR_XML_ERROR, "%s",
+ _("only a single TPM non-proxy device is
[PATCH v4 08/10] qemu: build command line for the TPM Proxy device
This patch wraps it up all the wiring done in previous patches,
enabling a PPC64 guest to launch a guest using a TPM Proxy
device.
Note that device validation is already being done in qemu_validate.c,
qemuValidateDomainDeviceDefTPM(), on domain define time. We don't
need to verify QEMU capabilities for this device again inside
qemu_command.c.
Reviewed-by: Stefan Berger
Signed-off-by: Daniel Henrique Barboza
---
src/qemu/qemu_alias.c | 5 -
src/qemu/qemu_command.c | 27 ++-
2 files changed, 30 insertions(+), 2 deletions(-)
diff --git a/src/qemu/qemu_alias.c b/src/qemu/qemu_alias.c
index 9d72391ddb..522426794d 100644
--- a/src/qemu/qemu_alias.c
+++ b/src/qemu/qemu_alias.c
@@ -408,7 +408,10 @@ qemuAssignDeviceTPMAlias(virDomainTPMDefPtr tpm,
if (tpm->info.alias)
return 0;
-tpm->info.alias = g_strdup_printf("tpm%d", idx);
+if (tpm->model == VIR_DOMAIN_TPM_MODEL_SPAPR_PROXY)
+tpm->info.alias = g_strdup_printf("tpmproxy%d", idx);
+else
+tpm->info.alias = g_strdup_printf("tpm%d", idx);
return 0;
}
diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
index d2ac19a7a8..1eef79b2a0 100644
--- a/src/qemu/qemu_command.c
+++ b/src/qemu/qemu_command.c
@@ -8979,6 +8979,26 @@ qemuBuildTPMCommandLine(virCommandPtr cmd,
}
+static int
+qemuBuildTPMProxyCommandLine(virCommandPtr cmd,
+ virDomainTPMDefPtr tpm)
+{
+g_auto(virBuffer) buf = VIR_BUFFER_INITIALIZER;
+const char *filePath = NULL;
+
+filePath = tpm->data.passthrough.source.data.file.path;
+
+virCommandAddArg(cmd, "-device");
+virBufferAsprintf(, "%s,id=%s,host-path=",
+ virDomainTPMModelTypeToString(tpm->model),
+ tpm->info.alias);
+virQEMUBuildBufferEscapeComma(, filePath);
+virCommandAddArgBuffer(cmd, );
+
+return 0;
+}
+
+
static int
qemuBuildTPMsCommandLine(virCommandPtr cmd,
const virDomainDef *def,
@@ -8987,8 +9007,13 @@ qemuBuildTPMsCommandLine(virCommandPtr cmd,
size_t i;
for (i = 0; i < def->ntpms; i++) {
-if (qemuBuildTPMCommandLine(cmd, def, def->tpms[i], qemuCaps) < 0)
+if (def->tpms[i]->model == VIR_DOMAIN_TPM_MODEL_SPAPR_PROXY) {
+if (qemuBuildTPMProxyCommandLine(cmd, def->tpms[i]) < 0)
+return -1;
+} else if (qemuBuildTPMCommandLine(cmd, def,
+ def->tpms[i], qemuCaps) < 0) {
return -1;
+}
}
return 0;
--
2.26.2
[PATCH v4 06/10] qemu_validate.c: add validation for TPM Proxy model
Previous patch handled the conversion of def->tpm to the
array def->tpms. What we're missing now is the validation
code to make the VIR_DOMAIN_TPM_MODEL_SPAPR_PROXY model
exclusive to PPC64 guests and VIR_DOMAIN_TPM_TYPE_PASSTHROUGH
backend.
Signed-off-by: Daniel Henrique Barboza
---
src/qemu/qemu_validate.c | 19 +++
1 file changed, 19 insertions(+)
diff --git a/src/qemu/qemu_validate.c b/src/qemu/qemu_validate.c
index 584d1375b8..28e02ebefc 100644
--- a/src/qemu/qemu_validate.c
+++ b/src/qemu/qemu_validate.c
@@ -3623,6 +3623,25 @@ qemuValidateDomainDeviceDefTPM(virDomainTPMDef *tpm,
case VIR_DOMAIN_TPM_MODEL_SPAPR:
flag = QEMU_CAPS_DEVICE_TPM_SPAPR;
break;
+case VIR_DOMAIN_TPM_MODEL_SPAPR_PROXY:
+if (!ARCH_IS_PPC64(def->os.arch)) {
+virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
+ _("TPM Proxy model %s is only available for "
+ "PPC64 guests"),
+ virDomainTPMModelTypeToString(tpm->model));
+return -1;
+}
+
+/* TPM Proxy devices have 'passthrough' backend */
+if (tpm->type != VIR_DOMAIN_TPM_TYPE_PASSTHROUGH) {
+virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
+ _("TPM Proxy model %s requires "
+ "'Passthrough' backend"),
+virDomainTPMModelTypeToString(tpm->model));
+}
+
+flag = QEMU_CAPS_DEVICE_SPAPR_TPM_PROXY;
+break;
case VIR_DOMAIN_TPM_MODEL_LAST:
default:
virReportEnumRangeError(virDomainTPMModel, tpm->model);
--
2.26.2
[PATCH v4 03/10] qemu_extdevice.c: remove unneeded 'ret' variable
qemuExtDevicesInitPaths() does not need 'ret'.
Signed-off-by: Daniel Henrique Barboza
---
src/qemu/qemu_extdevice.c | 6 ++
1 file changed, 2 insertions(+), 4 deletions(-)
diff --git a/src/qemu/qemu_extdevice.c b/src/qemu/qemu_extdevice.c
index 2096272761..2ff3f68f5d 100644
--- a/src/qemu/qemu_extdevice.c
+++ b/src/qemu/qemu_extdevice.c
@@ -73,12 +73,10 @@ static int
qemuExtDevicesInitPaths(virQEMUDriverPtr driver,
virDomainDefPtr def)
{
-int ret = 0;
-
if (def->tpm)
-ret = qemuExtTPMInitPaths(driver, def);
+return qemuExtTPMInitPaths(driver, def);
-return ret;
+return 0;
}
--
2.26.2
[PATCH v4 00/10] Introducing TPM Proxy device support for PPC64
changes in v4: - patch 1: added missing tags - comma-escaped the path string in qemu_command.c (patch 8) - moved validations not-XML-parsing related from domain_conf.c to qemu_validate.c - new patches 3 and 4: cleanups - patch 5 (former 3): changed the approach to use a def->tpms array instead of def->tpmproxy pointer - new patch 6: qemu_validate changes to not clog patch 5 with changes - added Jano's and Stefan's r-b when applicable v3 link: https://www.redhat.com/archives/libvir-list/2020-May/msg00642.html v2 link: https://www.redhat.com/archives/libvir-list/2020-May/msg00604.html v1 link: https://www.redhat.com/archives/libvir-list/2020-May/msg00604.html Daniel Henrique Barboza (10): docs: documentation and schema for the new TPM Proxy model qemu: Extend QEMU capabilities with 'spapr-tpm-proxy' qemu_extdevice.c: remove unneeded 'ret' variable qemu_tpm, security, tests: change 'switch' clauses for 'if' conf, qemu, security, tests: introducing 'def->tpms' array qemu_validate.c: add validation for TPM Proxy model tests: add XML schema tests for the TPM Proxy device qemu: build command line for the TPM Proxy device tests/qemuxml2argvtest.c: add TPM Proxy command line tests docs/news.xml: update for the new TPM Proxy device docs/formatdomain.html.in | 19 - docs/news.xml | 17 + docs/schemas/domaincommon.rng | 1 + src/conf/domain_audit.c | 4 +- src/conf/domain_conf.c| 72 +- src/conf/domain_conf.h| 6 +- src/qemu/qemu_alias.c | 9 ++- src/qemu/qemu_capabilities.c | 4 + src/qemu/qemu_capabilities.h | 3 + src/qemu/qemu_cgroup.c| 10 ++- src/qemu/qemu_command.c | 59 +++--- src/qemu/qemu_domain.c| 31 +--- src/qemu/qemu_domain_address.c| 11 ++- src/qemu/qemu_extdevice.c | 24 +++--- src/qemu/qemu_tpm.c | 76 +-- src/qemu/qemu_validate.c | 19 + src/security/security_dac.c | 8 +- src/security/security_selinux.c | 44 +-- src/security/virt-aa-helper.c | 14 ++-- .../qemucapabilitiesdata/caps_4.2.0.ppc64.xml | 1 + .../qemucapabilitiesdata/caps_5.0.0.ppc64.xml | 1 + tests/qemuxml2argvdata/ppc64-tpm-double.xml | 34 + .../ppc64-tpmproxy-double.xml | 38 ++ .../ppc64-tpmproxy-single.ppc64-latest.args | 34 + .../ppc64-tpmproxy-single.xml | 33 .../ppc64-tpmproxy-with-tpm.ppc64-latest.args | 37 + .../ppc64-tpmproxy-with-tpm.xml | 36 + tests/qemuxml2argvtest.c | 33 +--- .../ppc64-tpmproxy-single.ppc64-latest.xml| 42 ++ .../ppc64-tpmproxy-with-tpm.ppc64-latest.xml | 46 +++ tests/qemuxml2xmltest.c | 2 + 31 files changed, 616 insertions(+), 152 deletions(-) create mode 100644 tests/qemuxml2argvdata/ppc64-tpm-double.xml create mode 100644 tests/qemuxml2argvdata/ppc64-tpmproxy-double.xml create mode 100644 tests/qemuxml2argvdata/ppc64-tpmproxy-single.ppc64-latest.args create mode 100644 tests/qemuxml2argvdata/ppc64-tpmproxy-single.xml create mode 100644 tests/qemuxml2argvdata/ppc64-tpmproxy-with-tpm.ppc64-latest.args create mode 100644 tests/qemuxml2argvdata/ppc64-tpmproxy-with-tpm.xml create mode 100644 tests/qemuxml2xmloutdata/ppc64-tpmproxy-single.ppc64-latest.xml create mode 100644 tests/qemuxml2xmloutdata/ppc64-tpmproxy-with-tpm.ppc64-latest.xml -- 2.26.2
[PATCH v4 01/10] docs: documentation and schema for the new TPM Proxy model
QEMU 4.1.0 introduced a new device type called TPM Proxy, currently implemented by PPC64 guests via a new virtual device called 'spapr-tpm-proxy' (see QEMU 0fb6bd073230 for more info). The TPM Proxy device interacts with a TPM Resource Manager, a host device capable of multiplexing the host TPM with multiple processes. This allows multiple guests to access some TPM features at the same time. Note that this mode of operation does not provide full TPM features to be available for the guest - for that case the guest still needs to assign a vTPM device (tpm-spapr for PPC64 guests). Although redundant, there is currently no technical limitation for a guest to assign both a vTPM and a TPM Proxy at the same time. This patch adds documentation and schema for a new TPM model type called 'spapr-tpm-proxy' that creates this new TPM Proxy device. This model is valid only for the 'passthrough' backend. An example of a TPM Proxy device connected to a TPM Resource Manager '/dev/tpmrm0' will look like this: Signed-off-by: Daniel Henrique Barboza --- docs/formatdomain.html.in | 19 ++- docs/schemas/domaincommon.rng | 1 + 2 files changed, 19 insertions(+), 1 deletion(-) diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in index 23eb029234..15109e136c 100644 --- a/docs/formatdomain.html.in +++ b/docs/formatdomain.html.in @@ -8792,6 +8792,18 @@ qemu-kvm -net nic,model=? /dev/null backend device is a TPM 2.0. Since 6.1.0, pSeries guests on PPC64 are supported and the default is tpm-spapr. + + Since 6.4.0, a new model called + spapr-tpm-proxy was added for pSeries guests. This model + only works with the passthrough backend. It creates a + TPM Proxy device that communicates with an existing TPM Resource Manager + in the host, for example /dev/tpmrm0, enabling the guest to + run in secure virtual machine mode with the help of an Ultravisor. Adding + a TPM Proxy to a pSeries guest brings no security benefits unless the guest + is running on a PPC64 host that has an Ultravisor and a TPM Resource Manager. + Only one TPM Proxy device is allowed per guest, but a TPM Proxy device can + be added together with + other TPM devices. backend @@ -8804,7 +8816,7 @@ qemu-kvm -net nic,model=? /dev/null passthrough - Use the host's TPM device. + Use the host's TPM or TPM Resource Manager device. This backend type requires exclusive access to a TPM device on @@ -8812,6 +8824,11 @@ qemu-kvm -net nic,model=? /dev/null qualified file name is specified by path attribute of the source element. If no file name is specified then /dev/tpm0 is automatically used. + + Since 6.4.0, when choosing the + spapr-tpm-proxy model, the file name specified is + expected to be a TPM Resource Manager device, e.g. + /dev/tpmrm0. diff --git a/docs/schemas/domaincommon.rng b/docs/schemas/domaincommon.rng index 9d60b090f3..50860419c3 100644 --- a/docs/schemas/domaincommon.rng +++ b/docs/schemas/domaincommon.rng @@ -4610,6 +4610,7 @@ tpm-tis tpm-crb tpm-spapr +spapr-tpm-proxy -- 2.26.2
[PATCH v4 02/10] qemu: Extend QEMU capabilities with 'spapr-tpm-proxy'
Expose the TPM Proxy support for PPC64 guests by creating a new
cap called QEMU_CAPS_DEVICE_SPAPR_TPM_PROXY.
This device is part of the machinery the guest need to orchestrate
with the PPC64 Ultravisor the transition to the Secure VM (SVM)
mode. Inside QEMU, this device will be used with the H_TPM_COMM
hypercall to connect with the TPM Resource Manager, enabling
the guest to open and close TPM sessions with the host TPM.
Reviewed-by: Stefan Berger
Reviewed-by: Ján Tomko
Signed-off-by: Daniel Henrique Barboza
---
src/qemu/qemu_capabilities.c| 4
src/qemu/qemu_capabilities.h| 3 +++
tests/qemucapabilitiesdata/caps_4.2.0.ppc64.xml | 1 +
tests/qemucapabilitiesdata/caps_5.0.0.ppc64.xml | 1 +
4 files changed, 9 insertions(+)
diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c
index 7e711f22f8..d0d8b1ebf5 100644
--- a/src/qemu/qemu_capabilities.c
+++ b/src/qemu/qemu_capabilities.c
@@ -582,6 +582,9 @@ VIR_ENUM_IMPL(virQEMUCaps,
"tcg",
"virtio-blk-pci.scsi.default.disabled",
"pvscsi",
+
+ /* 370 */
+ "spapr-tpm-proxy",
);
@@ -1304,6 +1307,7 @@ struct virQEMUCapsStringFlags virQEMUCapsObjectTypes[] = {
{ "vhost-user-fs-device", QEMU_CAPS_DEVICE_VHOST_USER_FS },
{ "tcg-accel", QEMU_CAPS_TCG },
{ "pvscsi", QEMU_CAPS_SCSI_PVSCSI },
+{ "spapr-tpm-proxy", QEMU_CAPS_DEVICE_SPAPR_TPM_PROXY },
};
diff --git a/src/qemu/qemu_capabilities.h b/src/qemu/qemu_capabilities.h
index 6bfc7386e3..fa22856e12 100644
--- a/src/qemu/qemu_capabilities.h
+++ b/src/qemu/qemu_capabilities.h
@@ -564,6 +564,9 @@ typedef enum { /* virQEMUCapsFlags grouping marker for
syntax-check */
QEMU_CAPS_VIRTIO_BLK_SCSI_DEFAULT_DISABLED, /* virtio-blk-pci.scsi
disabled by default */
QEMU_CAPS_SCSI_PVSCSI, /* -device pvscsi */
+/* 370 */
+QEMU_CAPS_DEVICE_SPAPR_TPM_PROXY, /* -device spapr-tpm-proxy */
+
QEMU_CAPS_LAST /* this must always be the last item */
} virQEMUCapsFlags;
diff --git a/tests/qemucapabilitiesdata/caps_4.2.0.ppc64.xml
b/tests/qemucapabilitiesdata/caps_4.2.0.ppc64.xml
index a68786ddc8..9df68ebfc1 100644
--- a/tests/qemucapabilitiesdata/caps_4.2.0.ppc64.xml
+++ b/tests/qemucapabilitiesdata/caps_4.2.0.ppc64.xml
@@ -185,6 +185,7 @@
+
4001050
0
42900242
diff --git a/tests/qemucapabilitiesdata/caps_5.0.0.ppc64.xml
b/tests/qemucapabilitiesdata/caps_5.0.0.ppc64.xml
index c8cc07d954..77f51fe4d8 100644
--- a/tests/qemucapabilitiesdata/caps_5.0.0.ppc64.xml
+++ b/tests/qemucapabilitiesdata/caps_5.0.0.ppc64.xml
@@ -201,6 +201,7 @@
+
500
0
42900241
--
2.26.2
Re: [libvirt PATCH] qemu: prevent attempts to detach a device on a controller with hotplug='off'
On 5/15/20 3:58 AM, Erik Skultety wrote:
On Thu, May 14, 2020 at 02:12:38PM -0400, Laine Stump wrote:
Although the original patches to support controllers with
hotplug='off' were checking during hotplug/attach requests that the
device was being plugged into a PCI controller that didn't have
hotplug disabled, but I forgot to do the same for device detach (the
main impetus for adding the feature was to prevent unplugs originating
from within the guest, so it slipped my mind). So although the guest
OS was ultimately unable to honor the unplug request, libvirt could
still be used to make such a request, and since device attach/detach
are asynchronous operations, the caller to libvirt would receive a
success status back (the device would stubbornly/correctly remain in
the domain status XML however)
This patch remedies that, by looking at the controller for the device
in the detach request, and immediately failing the operation if that
controller has hotplug=off.
r changes. Lines starting
^This looks like some accidental copy-paste :)
Signed-off-by: Laine Stump
---
src/qemu/qemu_hotplug.c | 27 +++
1 file changed, 27 insertions(+)
diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c
index ab5a7aef84..5fe125b1a7 100644
--- a/src/qemu/qemu_hotplug.c
+++ b/src/qemu/qemu_hotplug.c
@@ -5891,6 +5891,33 @@ qemuDomainDetachDeviceLive(virDomainObjPtr vm,
return -1;
}
+if (info->type == VIR_DOMAIN_DEVICE_ADDRESS_TYPE_PCI) {
+int controllerIdx = virDomainControllerFind(vm->def,
+
VIR_DOMAIN_CONTROLLER_TYPE_PCI,
+info->addr.pci.bus);
+if (controllerIdx < 0) {
+virReportError(VIR_ERR_OPERATION_FAILED,
+ _("cannot hot unplug %s device with PCI guest address:
"
+ VIR_PCI_DEVICE_ADDRESS_FMT
+ " - controller not found"),
+ virDomainDeviceTypeToString(detach.type),
+ info->addr.pci.domain, info->addr.pci.bus,
+ info->addr.pci.slot, info->addr.pci.function);
+return -1;
+}
+
To enhance readability, use a temporary variable:
virDomainControllerDefPtr controller = vm->def->controllers[controllerIdx];
if (controller->opts.pciopts.hotplug == VIR_TRISTATE_SWITCH_OFF) {...}
I suppose it does put the two sides of the == on a single line. Other
than that it doesn't do much, just takes up an extra line because the
definition has to be on a separate line from its assignment (you can
only use controllerIdx if it's >= 0, so it could be invalid up in the
block where automatics need to be defined), and it's only used once (if
it was used more than once I would have already done it).
Anyway, since you asked, I did it now before pushing.
(NB: I also would have made a temporary variable to point directly at
info->addr.pci, but there was existing usage of it as is, and I would
have felt compelled to change that usage to use a temporary as well, but
didn't want to restructure that much. I'll send a followup patch for that)
+if (vm->def->controllers[controllerIdx]->opts.pciopts.hotplug
+== VIR_TRISTATE_SWITCH_OFF) {
+virReportError(VIR_ERR_OPERATION_FAILED,
So, VIR_ERR_OPERATION_FAILED makes perfect sense here, but it feels quite
inconsistent with what error we return in the opposite scenario when the flags
are compared in virDomainPCIAddressFlagsCompatible, where we'd report internal
error on hotplug.
That depends on whether the address was generated automatically by
libvirt, or provided by the user. virDomainPCIAddressFlagsCompatible
will give an INTERNAL_ERROR if libvirt generated the address (shouldn't
ever happen - libvirt should instead say that it can't find an
appropriate slot at an earlier time). If the user provides an address,
then it gives XML_ERROR. I guess at the time that code was written, we
were thinking more about the fact that the address came from XML written
by the user rather than what was being done with that XML.
Anyway, in the attach case, the user is saying "put this device *here*,
and libvirt is saying "*here* isn't a proper place to put this device".
In the detach case, the user is saying "unplug this device" and libvirt
is saying "you can't unplug this device", which is a bit of a different
thing from "this device can't be put here".
But that's just nitpicking. The more important thing is that I'm just
following whatever error types were already there for similar things
(and in a lot of cases the decision of which error type to use is
inaccurate and subjective, because the same function may be called from
different places that would suggest different error types - point in
case is that virDomainPCIAddressFlagsCompatible tries to use a different
error type
Re: [PATCH 05/21] qemuBuildChannelChrDeviceStr: Remove formatting of properties for -netdev
On 5/15/20 10:27 AM, Peter Krempa wrote: The output of the function is fed as argument to '-device' command line argument or 'device_add' monitor command except for 'guestfwd' channels where it needs to be fed to -netdev/netdev_add. This is confusing and error prone. Split it up since the caller needs to know which command/option to use anyways, so the caller can call the appropriate function without any magic. Signed-off-by: Peter Krempa --- src/qemu/qemu_command.c | 36 src/qemu/qemu_command.h | 3 +++ src/qemu/qemu_hotplug.c | 18 +- 3 files changed, 36 insertions(+), 21 deletions(-) Reviewed-by: Eric Blake -- Eric Blake, Principal Software Engineer Red Hat, Inc. +1-919-301-3226 Virtualization: qemu.org | libvirt.org
[PATCH trivial 1/1] qemu_alias.c: fix qemuAssingDeviceMemballoonAlias() typo
Signed-off-by: Daniel Henrique Barboza
---
src/qemu/qemu_alias.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/src/qemu/qemu_alias.c b/src/qemu/qemu_alias.c
index b0ea62af39..d6527cb046 100644
--- a/src/qemu/qemu_alias.c
+++ b/src/qemu/qemu_alias.c
@@ -390,7 +390,7 @@ qemuAssignDeviceSmartcardAlias(virDomainSmartcardDefPtr
smartcard,
static int
-qemuAssingDeviceMemballoonAlias(virDomainMemballoonDefPtr memballoon,
+qemuAssignDeviceMemballoonAlias(virDomainMemballoonDefPtr memballoon,
int idx)
{
if (memballoon->info.alias)
@@ -662,7 +662,7 @@ qemuAssignDeviceAliases(virDomainDefPtr def, virQEMUCapsPtr
qemuCaps)
}
if (def->memballoon &&
def->memballoon->model != VIR_DOMAIN_MEMBALLOON_MODEL_NONE) {
-if (qemuAssingDeviceMemballoonAlias(def->memballoon, 0) < 0)
+if (qemuAssignDeviceMemballoonAlias(def->memballoon, 0) < 0)
return -1;
}
for (i = 0; i < def->nrngs; i++) {
--
2.26.2
Re: [PATCH 04/21] qemuBuildChannelsCommandLine: Extract common formatting of 'chardev'
On 5/15/20 10:27 AM, Peter Krempa wrote: Both active branches create the same backend chardev. Since there is no other case, extract it before the switch so that we don't have to duplicate it. Signed-off-by: Peter Krempa --- src/qemu/qemu_command.c | 32 +++- 1 file changed, 11 insertions(+), 21 deletions(-) Reviewed-by: Eric Blake -- Eric Blake, Principal Software Engineer Red Hat, Inc. +1-919-301-3226 Virtualization: qemu.org | libvirt.org
Re: [PATCH 03/21] qemuBuildChannelsCommandLine: Use typecasted switch for channel type
On 5/15/20 10:27 AM, Peter Krempa wrote: Cover all cases of the enum. Signed-off-by: Peter Krempa --- src/qemu/qemu_command.c | 7 ++- 1 file changed, 6 insertions(+), 1 deletion(-) Reviewed-by: Eric Blake -- Eric Blake, Principal Software Engineer Red Hat, Inc. +1-919-301-3226 Virtualization: qemu.org | libvirt.org
Re: [PATCH 02/21] qemuMonitorJSONParseKeywords: remove constant argument
On 5/15/20 10:27 AM, Peter Krempa wrote: There's just one caller that always passes '1'. Signed-off-by: Peter Krempa --- src/qemu/qemu_monitor_json.c | 17 + 1 file changed, 5 insertions(+), 12 deletions(-) diff --git a/src/qemu/qemu_monitor_json.c b/src/qemu/qemu_monitor_json.c index 505b31a78a..66422a8489 100644 --- a/src/qemu/qemu_monitor_json.c +++ b/src/qemu/qemu_monitor_json.c @@ -604,16 +604,15 @@ qemuMonitorJSONParseKeywordsFree(int nkeywords, /* * Takes a string containing a set of key=value,key=value,key... * parameters and splits them up, returning two arrays with - * the individual keys and values. If allowEmptyValue is nonzero, - * the "=value" part is optional and if a key with no value is found, + * the individual keys and values. + * The "=value" part is optional and if a key with no value is found, * NULL is be placed into corresponding place in retvalues. While you are touching this, s/is be placed into/will be placed into the/ With that, Reviewed-by: Eric Blake -- Eric Blake, Principal Software Engineer Red Hat, Inc. +1-919-301-3226 Virtualization: qemu.org | libvirt.org
Re: [PATCH 01/21] qemu: domain: Forbid unsupported 'tftp' protocol and handle tests
On 5/15/20 10:27 AM, Peter Krempa wrote: 'tftp' storage protocol was supported by qemu until 2.7.0. Add an interlock when blockdev is used and drop the test case for it as it's IMO not worth adding another test file just for that. Fair enough. Signed-off-by: Peter Krempa --- src/qemu/qemu_domain.c | 10 ++ tests/qemuxml2argvdata/disk-cdrom-network.args | 3 --- .../disk-cdrom-network.x86_64-2.12.0.args | 3 --- .../disk-cdrom-network.x86_64-latest.args | 17 ++--- tests/qemuxml2argvdata/disk-cdrom-network.xml | 9 - 5 files changed, 16 insertions(+), 26 deletions(-) Reviewed-by: Eric Blake -- Eric Blake, Principal Software Engineer Red Hat, Inc. +1-919-301-3226 Virtualization: qemu.org | libvirt.org
Re: [PATCH 1/2] gitlab: introduce CI jobs for building content
On Fri, 2020-05-15 at 17:37 +0100, Daniel P. Berrangé wrote: > +ubuntu-2004-container: > + <<: *container_job_definition > + variables: > +NAME: ubuntu-2004 > + > + > + Way too many empty lines here :) > +# ubuntu-2004-docs is special as it is the one > +# we publish from > +pages: > + <<: *docs_job_definition > + variables: > +NAME: ubuntu-2004 > + artifacts: > +paths: > + - public It's kinda weird that this one has a different name despite being part of the same stage... Maybe ubuntu-2004-pages? Either way you're going to have to update the comment to reflect the actual name of the job. > +++ b/Makefile > @@ -3,17 +3,18 @@ prefix=/usr > datadir=$(prefix)/share > pkgdatadir=$(datadir)/publican > contentdir=$(pkgdatadir)/Common_Content > +branddir=$(contentdir) > > all: html pdf > > html: > - publican build --langs=en-US --formats=html > --common_content=$(contentdir) > + publican build --langs=en-US --formats=html > --common_content=$(contentdir) --brand_dir=$(branddir) > > pdf: > - publican build --langs=en-US --formats=pdf > --common_content=$(contentdir) > + publican build --langs=en-US --formats=pdf > --common_content=$(contentdir) --brand_dir=$(branddir) > > rpm: > - publican package --lang=en-US --binary --desktop > --common_content=$(contentdir) > + publican package --lang=en-US --binary --desktop > --common_content=$(contentdir) --brand_dir=$(branddir) > > clean: > publican clean --common_content=$(contentdir) Apologies for failing to point this out in v1, but these changes make sense on their own, so please split them off to a separate patch. Both for the new patch with the hunk above and the current one, Reviewed-by: Andrea Bolognani -- Andrea Bolognani / Red Hat / Virtualization
[PATCH 1/2] gitlab: introduce CI jobs for building content
The docs build needs to validate one axis - A variety of publican versions We get coverage for this by running builds across various distros. The Ubuntu 20.04 build is picked as the special one, from which we publish the generated HTML docs, which then become browsable via the GitLab Pages service. Signed-off-by: Daniel P. Berrangé --- .gitlab-ci.yml | 126 +++ Makefile | 7 +- ci/libvirt-centos-7.Dockerfile | 83 ++ ci/libvirt-debian-10.Dockerfile | 53 +++ ci/libvirt-debian-9.Dockerfile | 56 ci/libvirt-debian-sid.Dockerfile | 53 +++ ci/libvirt-fedora-31.Dockerfile | 50 +++ ci/libvirt-fedora-32.Dockerfile | 50 +++ ci/libvirt-fedora-rawhide.Dockerfile | 51 +++ ci/libvirt-ubuntu-1804.Dockerfile| 56 ci/libvirt-ubuntu-2004.Dockerfile| 53 +++ ci/refresh | 22 + 12 files changed, 657 insertions(+), 3 deletions(-) create mode 100644 ci/libvirt-centos-7.Dockerfile create mode 100644 ci/libvirt-debian-10.Dockerfile create mode 100644 ci/libvirt-debian-9.Dockerfile create mode 100644 ci/libvirt-debian-sid.Dockerfile create mode 100644 ci/libvirt-fedora-31.Dockerfile create mode 100644 ci/libvirt-fedora-32.Dockerfile create mode 100644 ci/libvirt-fedora-rawhide.Dockerfile create mode 100644 ci/libvirt-ubuntu-1804.Dockerfile create mode 100644 ci/libvirt-ubuntu-2004.Dockerfile create mode 100755 ci/refresh diff --git a/.gitlab-ci.yml b/.gitlab-ci.yml index 50dae92..450d844 100644 --- a/.gitlab-ci.yml +++ b/.gitlab-ci.yml @@ -1,6 +1,35 @@ stages: - prebuild + - containers + - docs + +.container_job_template: _job_definition + image: docker:stable + stage: containers + services: +- docker:dind + before_script: +- export TAG="$CI_REGISTRY_IMAGE/ci-$NAME:latest" +- export COMMON_TAG="$CI_REGISTRY/libvirt/libvirt-appdev-guide-python/ci-$NAME:latest" +- docker info +- docker login registry.gitlab.com -u "$CI_REGISTRY_USER" -p "$CI_REGISTRY_PASSWORD" + script: +- docker pull "$TAG" || docker pull "$COMMON_TAG" || true +- docker build --cache-from "$TAG" --cache-from "$COMMON_TAG" --tag "$TAG" -f "ci/libvirt-$NAME.Dockerfile" ci +- docker push "$TAG" + after_script: +- docker logout + +.docs_job_template: _job_definition + image: $CI_REGISTRY_IMAGE/ci-$NAME:latest + stage: docs + before_script: +- export MAKEFLAGS="-j$(getconf _NPROCESSORS_ONLN)" + script: +- git clone --depth 1 https://gitlab.com/libvirt/libvirt-publican.git brand +- $MAKE branddir=$PWD/brand +- mv tmp/en-US/html public # Check that all commits are signed-off for the DCO. # Skip on "libvirt" namespace, since we only need to run @@ -14,3 +43,100 @@ check-dco: except: variables: - $CI_PROJECT_NAMESPACE == 'libvirt' + +centos-7-container: + <<: *container_job_definition + variables: +NAME: centos-7 + +debian-9-container: + <<: *container_job_definition + variables: +NAME: debian-9 + +debian-10-container: + <<: *container_job_definition + variables: +NAME: debian-10 + +debian-sid-container: + <<: *container_job_definition + variables: +NAME: debian-sid + +fedora-31-container: + <<: *container_job_definition + variables: +NAME: fedora-31 + +fedora-32-container: + <<: *container_job_definition + variables: +NAME: fedora-32 + +fedora-rawhide-container: + <<: *container_job_definition + variables: +NAME: fedora-rawhide + +ubuntu-1804-container: + <<: *container_job_definition + variables: +NAME: ubuntu-1804 + +ubuntu-2004-container: + <<: *container_job_definition + variables: +NAME: ubuntu-2004 + + + +centos-7-docs: + <<: *docs_job_definition + variables: +NAME: centos-7 + +debian-9-docs: + <<: *docs_job_definition + variables: +NAME: debian-9 + +debian-10-docs: + <<: *docs_job_definition + variables: +NAME: debian-10 + +debian-sid-docs: + <<: *docs_job_definition + variables: +NAME: debian-sid + +fedora-31-docs: + <<: *docs_job_definition + variables: +NAME: fedora-31 + +fedora-32-docs: + <<: *docs_job_definition + variables: +NAME: fedora-32 + +fedora-rawhide-docs: + <<: *docs_job_definition + variables: +NAME: fedora-rawhide + +ubuntu-1804-docs: + <<: *docs_job_definition + variables: +NAME: ubuntu-1804 + +# ubuntu-2004-docs is special as it is the one +# we publish from +pages: + <<: *docs_job_definition + variables: +NAME: ubuntu-2004 + artifacts: +paths: + - public diff --git a/Makefile b/Makefile index 85935a1..237961a 100644 --- a/Makefile +++ b/Makefile @@ -3,17 +3,18 @@ prefix=/usr datadir=$(prefix)/share pkgdatadir=$(datadir)/publican contentdir=$(pkgdatadir)/Common_Content +branddir=$(contentdir) all: html pdf html: - publican build --langs=en-US --formats=html
[PATCH 2/2] gitlab: add CONTRIBUTING.rst file to indicate use of merge requests
With the introduction of automated CI pipelines, we are now ready to switch to using merge requests for the project. With this switch we longer wish to have patches sent to the mailing list. Reviewed-by: Andrea Bolognani Signed-off-by: Daniel P. Berrangé --- .gitpublish | 4 CONTRIBUTING.rst | 28 2 files changed, 28 insertions(+), 4 deletions(-) delete mode 100644 .gitpublish create mode 100644 CONTRIBUTING.rst diff --git a/.gitpublish b/.gitpublish deleted file mode 100644 index 1b77b6f..000 --- a/.gitpublish +++ /dev/null @@ -1,4 +0,0 @@ -[gitpublishprofile "default"] -base = master -to = libvir-list@redhat.com -prefix = libvirt-appdev-guide-python PATCH diff --git a/CONTRIBUTING.rst b/CONTRIBUTING.rst new file mode 100644 index 000..3679540 --- /dev/null +++ b/CONTRIBUTING.rst @@ -0,0 +1,28 @@ +=== +Contributing to libvirt-appdev-guide-python +=== + +The libvirt Python application developer guide accepts code contributions +via merge requests on the GitLab project: + +https://gitlab.com/libvirt/libvirt-appdev-guide-python/-/merge_requests + +It is required that automated CI pipelines succeed before a merge request +will be accepted. The global pipeline status for the ``master`` branch is +visible at: + +https://gitlab.com/libvirt/libvirt-appdev-guide-python/pipelines + +CI pipeline results for merge requests will be visible via the contributors' +own private repository fork: + +https://gitlab.com/yourusername/libvirt-appdev-guide-python/pipelines + +Contributions submitted to the project must be in compliance with the +Developer Certificate of Origin Version 1.1. This is documented at: + +https://developercertificate.org/ + +To indicate compliance, each commit in a series must have a "Signed-off-by" +tag with the submitter's name and email address. This can be added by passing +the ``-s`` flag to ``git commit`` when creating the patches. -- 2.26.2
[PATCH 0/2] Introduce GitLab CI and merge requests
In v2: - Use ubuntu for publishing pages build Daniel P. Berrangé (2): gitlab: introduce CI jobs for building content gitlab: add CONTRIBUTING.rst file to indicate use of merge requests .gitlab-ci.yml | 126 +++ .gitpublish | 4 - CONTRIBUTING.rst | 28 ++ Makefile | 7 +- ci/libvirt-centos-7.Dockerfile | 83 ++ ci/libvirt-debian-10.Dockerfile | 53 +++ ci/libvirt-debian-9.Dockerfile | 56 ci/libvirt-debian-sid.Dockerfile | 53 +++ ci/libvirt-fedora-31.Dockerfile | 50 +++ ci/libvirt-fedora-32.Dockerfile | 50 +++ ci/libvirt-fedora-rawhide.Dockerfile | 51 +++ ci/libvirt-ubuntu-1804.Dockerfile| 56 ci/libvirt-ubuntu-2004.Dockerfile| 53 +++ ci/refresh | 22 + 14 files changed, 685 insertions(+), 7 deletions(-) delete mode 100644 .gitpublish create mode 100644 CONTRIBUTING.rst create mode 100644 ci/libvirt-centos-7.Dockerfile create mode 100644 ci/libvirt-debian-10.Dockerfile create mode 100644 ci/libvirt-debian-9.Dockerfile create mode 100644 ci/libvirt-debian-sid.Dockerfile create mode 100644 ci/libvirt-fedora-31.Dockerfile create mode 100644 ci/libvirt-fedora-32.Dockerfile create mode 100644 ci/libvirt-fedora-rawhide.Dockerfile create mode 100644 ci/libvirt-ubuntu-1804.Dockerfile create mode 100644 ci/libvirt-ubuntu-2004.Dockerfile create mode 100755 ci/refresh -- 2.26.2
[PATCH] Don't require secdrivers to implement .domainMoveImageMetadata
The AppArmor secdriver does not use labels to grant access to
resources. Therefore, it doesn't use XATTRs and hence it lacks
implementation of .domainMoveImageMetadata callback. This leads
to a harmless but needless error message appearing in the logs:
virSecurityManagerMoveImageMetadata:476 : this function is not
supported by the connection driver: virSecurityManagerMoveImageMetadata
Closes: https://gitlab.com/libvirt/libvirt/-/issues/25
Signed-off-by: Michal Privoznik
---
src/security/security_manager.c | 3 +--
src/security/security_nop.c | 10 --
2 files changed, 1 insertion(+), 12 deletions(-)
diff --git a/src/security/security_manager.c b/src/security/security_manager.c
index 2dea294784..b1237d63b6 100644
--- a/src/security/security_manager.c
+++ b/src/security/security_manager.c
@@ -473,8 +473,7 @@ virSecurityManagerMoveImageMetadata(virSecurityManagerPtr
mgr,
return ret;
}
-virReportUnsupportedError();
-return -1;
+return 0;
}
diff --git a/src/security/security_nop.c b/src/security/security_nop.c
index c1856eb421..d5f715b916 100644
--- a/src/security/security_nop.c
+++ b/src/security/security_nop.c
@@ -225,15 +225,6 @@ virSecurityDomainSetImageLabelNop(virSecurityManagerPtr
mgr G_GNUC_UNUSED,
return 0;
}
-static int
-virSecurityDomainMoveImageMetadataNop(virSecurityManagerPtr mgr G_GNUC_UNUSED,
- pid_t pid G_GNUC_UNUSED,
- virStorageSourcePtr src G_GNUC_UNUSED,
- virStorageSourcePtr dst G_GNUC_UNUSED)
-{
-return 0;
-}
-
static int
virSecurityDomainSetMemoryLabelNop(virSecurityManagerPtr mgr G_GNUC_UNUSED,
virDomainDefPtr def G_GNUC_UNUSED,
@@ -290,7 +281,6 @@ virSecurityDriver virSecurityDriverNop = {
.domainSetSecurityImageLabel= virSecurityDomainSetImageLabelNop,
.domainRestoreSecurityImageLabel=
virSecurityDomainRestoreImageLabelNop,
-.domainMoveImageMetadata=
virSecurityDomainMoveImageMetadataNop,
.domainSetSecurityMemoryLabel = virSecurityDomainSetMemoryLabelNop,
.domainRestoreSecurityMemoryLabel =
virSecurityDomainRestoreMemoryLabelNop,
--
2.26.2
[PATCH 18/21] qemu: Prepare for testing of 'netdev_add' props via qemuxml2argvtest
qemuxml2argv test suite is way more comprehensive than the hotplug
suite. Since we share the code paths for monitor and command line
hotplug we can easily test the properties of devices against the QAPI
schema.
To achieve this we'll need to skip the JSON->commandline conversion for
the test run so that we can analyze the pure properties. This patch adds
flags for the comand line generator and hook them into the
JSON->commandline convertor for -netdev. An upcoming patch will make use
of this new infrastructure.
Signed-off-by: Peter Krempa
---
src/qemu/qemu_command.c | 28 +---
src/qemu/qemu_command.h | 7 ++-
src/qemu/qemu_driver.c | 2 +-
src/qemu/qemu_process.c | 11 +--
src/qemu/qemu_process.h | 1 +
src/util/virqemu.c | 9 -
src/util/virqemu.h | 3 ++-
tests/qemuxml2argvtest.c | 6 --
8 files changed, 48 insertions(+), 19 deletions(-)
diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
index 2ed8e66f66..f9a2732587 100644
--- a/src/qemu/qemu_command.c
+++ b/src/qemu/qemu_command.c
@@ -7691,7 +7691,8 @@ qemuBuildInterfaceCommandLine(virQEMUDriverPtr driver,
virNetDevVPortProfileOp vmop,
bool standalone,
size_t *nnicindexes,
- int **nicindexes)
+ int **nicindexes,
+ unsigned int flags)
{
virDomainDefPtr def = vm->def;
int ret = -1;
@@ -7920,7 +7921,8 @@ qemuBuildInterfaceCommandLine(virQEMUDriverPtr driver,
slirpfdName)))
goto cleanup;
-if (!(host = virQEMUBuildNetdevCommandlineFromJSON(hostnetprops)))
+if (!(host = virQEMUBuildNetdevCommandlineFromJSON(hostnetprops,
+ (flags &
QEMU_BUILD_COMMANDLINE_VALIDATE_KEEP_JSON
goto cleanup;
virCommandAddArgList(cmd, "-netdev", host, NULL);
@@ -7996,7 +7998,8 @@ qemuBuildNetCommandLine(virQEMUDriverPtr driver,
bool standalone,
size_t *nnicindexes,
int **nicindexes,
-unsigned int *bootHostdevNet)
+unsigned int *bootHostdevNet,
+unsigned int flags)
{
size_t i;
int last_good_net = -1;
@@ -8020,7 +8023,7 @@ qemuBuildNetCommandLine(virQEMUDriverPtr driver,
if (qemuBuildInterfaceCommandLine(driver, vm, logManager,
secManager, cmd, net,
qemuCaps, bootNet, vmop,
standalone, nnicindexes,
- nicindexes) < 0)
+ nicindexes, flags) < 0)
goto error;
last_good_net = i;
@@ -8556,7 +8559,8 @@ qemuBuildChannelsCommandLine(virLogManagerPtr logManager,
virQEMUDriverConfigPtr cfg,
const virDomainDef *def,
virQEMUCapsPtr qemuCaps,
- bool chardevStdioLogd)
+ bool chardevStdioLogd,
+ unsigned int flags)
{
size_t i;
unsigned int cdevflags = QEMU_BUILD_CHARDEV_TCP_NOWAIT |
@@ -8585,7 +8589,8 @@ qemuBuildChannelsCommandLine(virLogManagerPtr logManager,
if (!(netdevprops = qemuBuildChannelGuestfwdNetdevProps(channel)))
return -1;
-if (!(netdevstr =
virQEMUBuildNetdevCommandlineFromJSON(netdevprops)))
+if (!(netdevstr =
virQEMUBuildNetdevCommandlineFromJSON(netdevprops,
+(flags &
QEMU_BUILD_COMMANDLINE_VALIDATE_KEEP_JSON
return -1;
virCommandAddArgList(cmd, "-netdev", netdevstr, NULL);
@@ -9521,7 +9526,8 @@ qemuBuildCommandLine(virQEMUDriverPtr driver,
bool standalone,
bool enableFips,
size_t *nnicindexes,
- int **nicindexes)
+ int **nicindexes,
+ unsigned int flags)
{
size_t i;
char uuid[VIR_UUID_STRING_BUFLEN];
@@ -9534,9 +9540,9 @@ qemuBuildCommandLine(virQEMUDriverPtr driver,
bool chardevStdioLogd = priv->chardevStdioLogd;
VIR_DEBUG("driver=%p def=%p mon=%p "
- "qemuCaps=%p migrateURI=%s snapshot=%p vmop=%d",
+ "qemuCaps=%p migrateURI=%s snapshot=%p vmop=%d flags=0x%x",
driver, def, priv->monConfig,
- qemuCaps, migrateURI, snapshot, vmop);
+ qemuCaps, migrateURI, snapshot, vmop, flags);
if (qemuBuildCommandLineValidate(driver, def) < 0)
return NULL;
@@ -9681,7 +9687,7 @@ qemuBuildCommandLine(virQEMUDriverPtr driver,
if
[PATCH 17/21] qemuMonitorAddNetdev: Convert to the native JSON props object
Now that all code paths generate JSON props we can remove the conversion
to command line arguments and back in the monitor code.
Note that the test which is removed in this commit will be replaced by a
stronger testsuite later.
Signed-off-by: Peter Krempa
---
src/qemu/qemu_hotplug.c | 14 +++---
src/qemu/qemu_monitor.c | 8
src/qemu/qemu_monitor.h | 2 +-
src/qemu/qemu_monitor_json.c | 15 +++
src/qemu/qemu_monitor_json.h | 2 +-
tests/qemumonitorjsontest.c | 2 --
6 files changed, 12 insertions(+), 31 deletions(-)
diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c
index cd9bc9b2f8..2b0cdad4e7 100644
--- a/src/qemu/qemu_hotplug.c
+++ b/src/qemu/qemu_hotplug.c
@@ -1159,7 +1159,6 @@ qemuDomainAttachNetDevice(virQEMUDriverPtr driver,
size_t queueSize = 0;
g_autofree char *nicstr = NULL;
g_autoptr(virJSONValue) netprops = NULL;
-g_autofree char *netstr = NULL;
int ret = -1;
bool releaseaddr = false;
bool iface_connected = false;
@@ -1390,9 +1389,6 @@ qemuDomainAttachNetDevice(virQEMUDriverPtr driver,
slirpfdName)))
goto cleanup;
-if (!(netstr = virQEMUBuildNetdevCommandlineFromJSON(netprops)))
-goto cleanup;
-
qemuDomainObjEnterMonitor(driver, vm);
if (actualType == VIR_DOMAIN_NET_TYPE_VHOSTUSER) {
@@ -1404,7 +1400,7 @@ qemuDomainAttachNetDevice(virQEMUDriverPtr driver,
charDevPlugged = true;
}
-if (qemuMonitorAddNetdev(priv->mon, netstr,
+if (qemuMonitorAddNetdev(priv->mon, ,
tapfd, tapfdName, tapfdSize,
vhostfd, vhostfdName, vhostfdSize,
slirpfd, slirpfdName) < 0) {
@@ -2114,7 +2110,6 @@ int qemuDomainAttachChrDevice(virQEMUDriverPtr driver,
virDomainDefPtr vmdef = vm->def;
g_autofree char *devstr = NULL;
g_autoptr(virJSONValue) netdevprops = NULL;
-g_autofree char *netdevstr = NULL;
virDomainChrSourceDefPtr dev = chr->source;
g_autofree char *charAlias = NULL;
bool chardevAttached = false;
@@ -2156,9 +2151,6 @@ int qemuDomainAttachChrDevice(virQEMUDriverPtr driver,
if (guestfwd) {
if (!(netdevprops = qemuBuildChannelGuestfwdNetdevProps(chr)))
goto cleanup;
-
-if (!(netdevstr = virQEMUBuildNetdevCommandlineFromJSON(netdevprops)))
-goto cleanup;
} else {
if (qemuBuildChrDeviceStr(, vmdef, chr, priv->qemuCaps) < 0)
goto cleanup;
@@ -2181,8 +2173,8 @@ int qemuDomainAttachChrDevice(virQEMUDriverPtr driver,
goto exit_monitor;
chardevAttached = true;
-if (netdevstr) {
-if (qemuMonitorAddNetdev(priv->mon, netdevstr,
+if (netdevprops) {
+if (qemuMonitorAddNetdev(priv->mon, ,
NULL, NULL, 0, NULL, NULL, 0, -1, NULL) < 0)
goto exit_monitor;
}
diff --git a/src/qemu/qemu_monitor.c b/src/qemu/qemu_monitor.c
index 9c853ccb93..2911307f0e 100644
--- a/src/qemu/qemu_monitor.c
+++ b/src/qemu/qemu_monitor.c
@@ -2667,7 +2667,7 @@ qemuMonitorCloseFileHandle(qemuMonitorPtr mon,
int
qemuMonitorAddNetdev(qemuMonitorPtr mon,
- const char *netdevstr,
+ virJSONValuePtr *props,
int *tapfd, char **tapfdName, int tapfdSize,
int *vhostfd, char **vhostfdName, int vhostfdSize,
int slirpfd, char *slirpfdName)
@@ -2675,10 +2675,10 @@ qemuMonitorAddNetdev(qemuMonitorPtr mon,
int ret = -1;
size_t i = 0, j = 0;
-VIR_DEBUG("netdevstr=%s tapfd=%p tapfdName=%p tapfdSize=%d"
+VIR_DEBUG("props=%p tapfd=%p tapfdName=%p tapfdSize=%d"
"vhostfd=%p vhostfdName=%p vhostfdSize=%d"
"slirpfd=%d slirpfdName=%s",
- netdevstr, tapfd, tapfdName, tapfdSize,
+ props, tapfd, tapfdName, tapfdSize,
vhostfd, vhostfdName, vhostfdSize, slirpfd, slirpfdName);
QEMU_CHECK_MONITOR(mon);
@@ -2696,7 +2696,7 @@ qemuMonitorAddNetdev(qemuMonitorPtr mon,
qemuMonitorSendFileHandle(mon, slirpfdName, slirpfd) < 0)
goto cleanup;
-ret = qemuMonitorJSONAddNetdev(mon, netdevstr);
+ret = qemuMonitorJSONAddNetdev(mon, props);
cleanup:
if (ret < 0) {
diff --git a/src/qemu/qemu_monitor.h b/src/qemu/qemu_monitor.h
index 2e35d94bda..378e2532c7 100644
--- a/src/qemu/qemu_monitor.h
+++ b/src/qemu/qemu_monitor.h
@@ -881,7 +881,7 @@ int qemuMonitorCloseFileHandle(qemuMonitorPtr mon,
const char *fdname);
int qemuMonitorAddNetdev(qemuMonitorPtr mon,
- const char *netdevstr,
+ virJSONValuePtr *props,
int *tapfd, char **tapfdName, int tapfdSize,
int *vhostfd, char **vhostfdName, int vhostfdSize,
int slirpfd, char *slirpfdName);
[PATCH 15/21] virQEMUBuildNetdevCommandlineFromJSON: Prepare for quirky 'guestfwd'
QEMU models guestfwd as:
'guestfwd': [
{ "str": "tcp:10.0.2.1:4600-chardev:charchannel0" },
{ "str": ""},
]
I guess the original idea was to make it extensible while not worrying
about adding another object for it. Either way it requires us to add yet
another JSON->cmdline convertor for arrays.
Signed-off-by: Peter Krempa
---
src/util/virqemu.c | 38 +-
1 file changed, 37 insertions(+), 1 deletion(-)
diff --git a/src/util/virqemu.c b/src/util/virqemu.c
index 0f8cab29df..f3e9966598 100644
--- a/src/util/virqemu.c
+++ b/src/util/virqemu.c
@@ -109,6 +109,41 @@ virQEMUBuildCommandLineJSONArrayNumbered(const char *key,
}
+/**
+ * This array convertor is for quirky cases where the QMP schema mandates an
+ * array of objects with only one attribute 'str' which needs to be formatted
as
+ * repeated key-value pairs without the 'str' being printed:
+ *
+ * 'guestfwd': [
+ * { "str": "tcp:10.0.2.1:4600-chardev:charchannel0" },
+ * { "str": ""},
+ * ]
+ */
+static int
+virQEMUBuildCommandLineJSONArrayObjectsStr(const char *key,
+ virJSONValuePtr array,
+ virBufferPtr buf,
+ const char *skipKey G_GNUC_UNUSED,
+ bool onOff G_GNUC_UNUSED)
+{
+g_auto(virBuffer) tmp = VIR_BUFFER_INITIALIZER;
+size_t i;
+
+for (i = 0; i < virJSONValueArraySize(array); i++) {
+virJSONValuePtr member = virJSONValueArrayGet(array, i);
+const char *str = virJSONValueObjectGetString(member, "str");
+
+if (!str)
+return -1;
+
+virBufferAsprintf(, "%s=%s,", key, str);
+}
+
+virBufferAddBuffer(buf, );
+return 0;
+}
+
+
/* internal iterator to handle nested object formatting */
static int
virQEMUBuildCommandLineJSONIterate(const char *key,
@@ -267,7 +302,8 @@ virQEMUBuildNetdevCommandlineFromJSON(virJSONValuePtr props)
virBufferAsprintf(, "%s,", type);
-if (virQEMUBuildCommandLineJSON(props, , "type", true, NULL) < 0)
+if (virQEMUBuildCommandLineJSON(props, , "type", true,
+
virQEMUBuildCommandLineJSONArrayObjectsStr) < 0)
return NULL;
return virBufferContentAndReset();
--
2.26.2
[PATCH 20/21] testutilsqemuschema: Allow loading non-latest schema
Add testQEMUSchemaLoad and refactor internals so that we can load the
QMP schema from an arbitrary caps replies file.
Signed-off-by: Peter Krempa
---
tests/testutilsqemuschema.c | 64 -
tests/testutilsqemuschema.h | 3 ++
2 files changed, 45 insertions(+), 22 deletions(-)
diff --git a/tests/testutilsqemuschema.c b/tests/testutilsqemuschema.c
index 060a5d7054..d9b55fd787 100644
--- a/tests/testutilsqemuschema.c
+++ b/tests/testutilsqemuschema.c
@@ -607,37 +607,23 @@ testQEMUSchemaValidateCommand(const char *command,
}
-/**
- * testQEMUSchemaGetLatest:
- *
- * Returns the schema data as the qemu monitor would reply from the latest
- * replies file used for qemucapabilitiestest for the x86_64 architecture.
- */
-virJSONValuePtr
-testQEMUSchemaGetLatest(const char* arch)
+static virJSONValuePtr
+testQEMUSchemaLoadReplies(const char *filename)
{
-g_autofree char *capsLatestFile = NULL;
-g_autofree char *capsLatest = NULL;
+g_autofree char *caps = NULL;
char *schemaReply;
char *end;
g_autoptr(virJSONValue) reply = NULL;
virJSONValuePtr schema = NULL;
-if (!(capsLatestFile = testQemuGetLatestCapsForArch(arch, "replies"))) {
-VIR_TEST_VERBOSE("failed to find latest caps replies");
+if (virTestLoadFile(filename, ) < 0)
return NULL;
-}
-VIR_TEST_DEBUG("replies file: '%s'", capsLatestFile);
-
-if (virTestLoadFile(capsLatestFile, ) < 0)
-return NULL;
-
-if (!(schemaReply = strstr(capsLatest, "\"execute\":
\"query-qmp-schema\"")) ||
+if (!(schemaReply = strstr(caps, "\"execute\": \"query-qmp-schema\"")) ||
!(schemaReply = strstr(schemaReply, "\n\n")) ||
!(end = strstr(schemaReply + 2, "\n\n"))) {
VIR_TEST_VERBOSE("failed to find reply to 'query-qmp-schema' in '%s'",
- capsLatestFile);
+ filename);
return NULL;
}
@@ -646,13 +632,13 @@ testQEMUSchemaGetLatest(const char* arch)
if (!(reply = virJSONValueFromString(schemaReply))) {
VIR_TEST_VERBOSE("failed to parse 'query-qmp-schema' reply from '%s'",
- capsLatestFile);
+ filename);
return NULL;
}
if (!(schema = virJSONValueObjectStealArray(reply, "return"))) {
VIR_TEST_VERBOSE("missing qapi schema data in reply in '%s'",
- capsLatestFile);
+ filename);
return NULL;
}
@@ -660,6 +646,28 @@ testQEMUSchemaGetLatest(const char* arch)
}
+/**
+ * testQEMUSchemaGetLatest:
+ *
+ * Returns the schema data as the qemu monitor would reply from the latest
+ * replies file used for qemucapabilitiestest for the x86_64 architecture.
+ */
+virJSONValuePtr
+testQEMUSchemaGetLatest(const char *arch)
+{
+g_autofree char *capsLatestFile = NULL;
+
+if (!(capsLatestFile = testQemuGetLatestCapsForArch(arch, "replies"))) {
+VIR_TEST_VERBOSE("failed to find latest caps replies");
+return NULL;
+}
+
+VIR_TEST_DEBUG("replies file: '%s'", capsLatestFile);
+
+return testQEMUSchemaLoadReplies(capsLatestFile);
+}
+
+
virHashTablePtr
testQEMUSchemaLoadLatest(const char *arch)
{
@@ -670,3 +678,15 @@ testQEMUSchemaLoadLatest(const char *arch)
return virQEMUQAPISchemaConvert(schema);
}
+
+
+virHashTablePtr
+testQEMUSchemaLoad(const char *filename)
+{
+virJSONValuePtr schema;
+
+if (!(schema = testQEMUSchemaLoadReplies(filename)))
+return NULL;
+
+return virQEMUQAPISchemaConvert(schema);
+}
diff --git a/tests/testutilsqemuschema.h b/tests/testutilsqemuschema.h
index acedb77677..c90a6b626d 100644
--- a/tests/testutilsqemuschema.h
+++ b/tests/testutilsqemuschema.h
@@ -42,3 +42,6 @@ testQEMUSchemaGetLatest(const char* arch);
virHashTablePtr
testQEMUSchemaLoadLatest(const char *arch);
+
+virHashTablePtr
+testQEMUSchemaLoad(const char *filename);
--
2.26.2
[PATCH 16/21] qemuBuildChannelGuestfwdNetdevProps: Convert to generating JSON props
Syntax of guestfwd channel also needs to be modified to conform to the
QAPI schema.
Signed-off-by: Peter Krempa
---
src/qemu/qemu_command.c | 37 +++--
src/qemu/qemu_command.h | 2 +-
src/qemu/qemu_hotplug.c | 6 +-
3 files changed, 37 insertions(+), 8 deletions(-)
diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
index 00d1b4121d..2ed8e66f66 100644
--- a/src/qemu/qemu_command.c
+++ b/src/qemu/qemu_command.c
@@ -8567,6 +8567,7 @@ qemuBuildChannelsCommandLine(virLogManagerPtr logManager,
for (i = 0; i < def->nchannels; i++) {
virDomainChrDefPtr channel = def->channels[i];
g_autofree char *chardevstr = NULL;
+g_autoptr(virJSONValue) netdevprops = NULL;
g_autofree char *netdevstr = NULL;
if (!(chardevstr = qemuBuildChrChardevStr(logManager, secManager,
@@ -8581,8 +8582,12 @@ qemuBuildChannelsCommandLine(virLogManagerPtr logManager,
switch ((virDomainChrChannelTargetType) channel->targetType) {
case VIR_DOMAIN_CHR_CHANNEL_TARGET_TYPE_GUESTFWD:
-if (!(netdevstr = qemuBuildChannelGuestfwdNetdevProps(channel)))
+if (!(netdevprops = qemuBuildChannelGuestfwdNetdevProps(channel)))
return -1;
+
+if (!(netdevstr =
virQEMUBuildNetdevCommandlineFromJSON(netdevprops)))
+return -1;
+
virCommandAddArgList(cmd, "-netdev", netdevstr, NULL);
break;
@@ -9850,19 +9855,39 @@ qemuBuildParallelChrDeviceStr(char **deviceStr,
}
-char *
+virJSONValuePtr
qemuBuildChannelGuestfwdNetdevProps(virDomainChrDefPtr chr)
{
+g_autoptr(virJSONValue) guestfwdarr = virJSONValueNewArray();
+g_autoptr(virJSONValue) guestfwdstrobj = virJSONValueNewObject();
g_autofree char *addr = NULL;
-int port;
+virJSONValuePtr ret = NULL;
if (!(addr = virSocketAddrFormat(chr->target.addr)))
return NULL;
-port = virSocketAddrGetPort(chr->target.addr);
+/* this may seem weird, but qemu indeed decided that 'guestfwd' parameter
+ * is an array of objects which have just one member named 'str' which
+ * contains the description */
+if (virJSONValueObjectAppendStringPrintf(guestfwdstrobj, "str",
+ "tcp:%s:%i-chardev:char%s",
+ addr,
+
virSocketAddrGetPort(chr->target.addr),
+ chr->info.alias) < 0)
+return NULL;
+
+if (virJSONValueArrayAppend(guestfwdarr, guestfwdstrobj) < 0)
+return NULL;
+guestfwdstrobj = NULL;
-return g_strdup_printf("user,guestfwd=tcp:%s:%i-chardev:char%s,id=%s",
- addr, port, chr->info.alias, chr->info.alias);
+if (virJSONValueObjectCreate(,
+ "s:type", "user",
+ "a:guestfwd", ,
+ "s:id", chr->info.alias,
+ NULL) < 0)
+return NULL;
+
+return ret;
}
diff --git a/src/qemu/qemu_command.h b/src/qemu/qemu_command.h
index 6cdd9debe0..1daa07982c 100644
--- a/src/qemu/qemu_command.h
+++ b/src/qemu/qemu_command.h
@@ -86,7 +86,7 @@ qemuBuildChrDeviceStr(char **deviceStr,
virDomainChrDefPtr chr,
virQEMUCapsPtr qemuCaps);
-char *
+virJSONValuePtr
qemuBuildChannelGuestfwdNetdevProps(virDomainChrDefPtr chr);
virJSONValuePtr qemuBuildHostNetStr(virDomainNetDefPtr net,
diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c
index c6789dcef3..cd9bc9b2f8 100644
--- a/src/qemu/qemu_hotplug.c
+++ b/src/qemu/qemu_hotplug.c
@@ -2113,6 +2113,7 @@ int qemuDomainAttachChrDevice(virQEMUDriverPtr driver,
virErrorPtr orig_err;
virDomainDefPtr vmdef = vm->def;
g_autofree char *devstr = NULL;
+g_autoptr(virJSONValue) netdevprops = NULL;
g_autofree char *netdevstr = NULL;
virDomainChrSourceDefPtr dev = chr->source;
g_autofree char *charAlias = NULL;
@@ -2153,7 +2154,10 @@ int qemuDomainAttachChrDevice(virQEMUDriverPtr driver,
teardowncgroup = true;
if (guestfwd) {
-if (!(netdevstr = qemuBuildChannelGuestfwdNetdevProps(chr)))
+if (!(netdevprops = qemuBuildChannelGuestfwdNetdevProps(chr)))
+goto cleanup;
+
+if (!(netdevstr = virQEMUBuildNetdevCommandlineFromJSON(netdevprops)))
goto cleanup;
} else {
if (qemuBuildChrDeviceStr(, vmdef, chr, priv->qemuCaps) < 0)
--
2.26.2
[PATCH 14/21] qemu: command: Generate -netdev command line via JSON->cmdline conversion
The 'netdev_add' command was recently formally described in qemu via the
QMP schema. This means that it also requires the arguments to be
properly formatted. Our current approach is to generate the command line
and then use qemuMonitorJSONKeywordStringToJSON to get the JSON
properties for the monitor. This will not work if we need to pass some
fields as numbers or booleans.
In this step we re-do internals of qemuBuildHostNetStr to format a JSON
object which is converted back via virQEMUBuildNetdevCommandlineFromJSON
to the equivalent command line. This will later allow fixing of the
monitor code to use the JSON object directly rather than rely on the
conversion.
Signed-off-by: Peter Krempa
---
src/qemu/qemu_command.c | 163 +---
src/qemu/qemu_command.h | 12 +--
src/qemu/qemu_hotplug.c | 13 +++-
3 files changed, 119 insertions(+), 69 deletions(-)
diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
index 2850953bd0..00d1b4121d 100644
--- a/src/qemu/qemu_command.c
+++ b/src/qemu/qemu_command.c
@@ -3564,7 +3564,7 @@ qemuBuildNicDevStr(virDomainDefPtr def,
}
-char *
+virJSONValuePtr
qemuBuildHostNetStr(virDomainNetDefPtr net,
char **tapfd,
size_t tapfdSize,
@@ -3573,10 +3573,11 @@ qemuBuildHostNetStr(virDomainNetDefPtr net,
const char *slirpfd)
{
bool is_tap = false;
-g_auto(virBuffer) buf = VIR_BUFFER_INITIALIZER;
virDomainNetType netType = virDomainNetGetActualType(net);
size_t i;
+g_autoptr(virJSONValue) netprops = NULL;
+
if (net->script && netType != VIR_DOMAIN_NET_TYPE_ETHERNET) {
virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
_("scripts are not supported on interfaces of type %s"),
@@ -3594,54 +3595,75 @@ qemuBuildHostNetStr(virDomainNetDefPtr net,
case VIR_DOMAIN_NET_TYPE_NETWORK:
case VIR_DOMAIN_NET_TYPE_DIRECT:
case VIR_DOMAIN_NET_TYPE_ETHERNET:
-virBufferAddLit(, "tap,");
+if (virJSONValueObjectCreate(, "s:type", "tap", NULL) < 0)
+return NULL;
+
/* for one tapfd 'fd=' shall be used,
* for more than one 'fds=' is the right choice */
if (tapfdSize == 1) {
-virBufferAsprintf(, "fd=%s,", tapfd[0]);
+if (virJSONValueObjectAdd(netprops, "s:fd", tapfd[0], NULL) < 0)
+return NULL;
} else {
-virBufferAddLit(, "fds=");
-for (i = 0; i < tapfdSize; i++) {
-if (i)
-virBufferAddChar(, ':');
-virBufferAdd(, tapfd[i], -1);
-}
-virBufferAddChar(, ',');
+g_auto(virBuffer) fdsbuf = VIR_BUFFER_INITIALIZER;
+
+for (i = 0; i < tapfdSize; i++)
+virBufferAsprintf(, "%s:", tapfd[i]);
+
+virBufferTrim(, ":");
+
+if (virJSONValueObjectAdd(netprops,
+ "s:fds",
virBufferCurrentContent(),
+ NULL) < 0)
+return NULL;
}
+
is_tap = true;
break;
case VIR_DOMAIN_NET_TYPE_CLIENT:
-virBufferAsprintf(, "socket,connect=%s:%d,",
- net->data.socket.address,
- net->data.socket.port);
+if (virJSONValueObjectCreate(, "s:type", "socket", NULL) < 0
||
+virJSONValueObjectAppendStringPrintf(netprops, "connect", "%s:%d",
+ net->data.socket.address,
+ net->data.socket.port) < 0)
+return NULL;
break;
case VIR_DOMAIN_NET_TYPE_SERVER:
-virBufferAsprintf(, "socket,listen=%s:%d,",
- NULLSTR_EMPTY(net->data.socket.address),
- net->data.socket.port);
+if (virJSONValueObjectCreate(, "s:type", "socket", NULL) < 0
||
+virJSONValueObjectAppendStringPrintf(netprops, "listen", "%s:%d",
+
NULLSTR_EMPTY(net->data.socket.address),
+ net->data.socket.port) < 0)
+return NULL;
break;
case VIR_DOMAIN_NET_TYPE_MCAST:
-virBufferAsprintf(, "socket,mcast=%s:%d,",
- net->data.socket.address,
- net->data.socket.port);
+if (virJSONValueObjectCreate(, "s:type", "socket", NULL) < 0
||
+virJSONValueObjectAppendStringPrintf(netprops, "mcast", "%s:%d",
+ net->data.socket.address,
+ net->data.socket.port) < 0)
+return NULL;
break;
case VIR_DOMAIN_NET_TYPE_UDP:
-virBufferAsprintf(, "socket,udp=%s:%d,localaddr=%s:%d,",
- net->data.socket.address,
-
Re: Firmware auto-select limitation
On Fri, May 15, 2020 at 17:16:57 +0200, Michal Privoznik wrote: > On 5/15/20 4:40 PM, GUOQING LI wrote: [...] > > *Detailed error * > > Error starting domain: internal error: process exited while connecting > > to monitor: 2020-05-15T14:19:06.033267Z qemu-system-x86_64: -drive > > file=/usr/share/OVMF/OVMF_CODE.fd,if=pflash,format=raw,unit=0,readonly=on: > > Failed to lock byte 100 > > > > This error message comes from QEMU. Unfortunately, it doesn't say why > locking the file failed. Is there perhaps some additional info in the audit > log? IMO there's another qemu where the 'readonly=on' command line property or thus the property in the XML is missing and thus it's opened in write mode. Readers are forbidden then.
[PATCH 19/21] testQEMUSchemaLoad: Rename to testQEMUSchemaLoadLatest
It always loads the latest schema. Prepare for loading others as well.
Signed-off-by: Peter Krempa
---
tests/qemublocktest.c | 2 +-
tests/qemuhotplugtest.c | 2 +-
tests/qemumonitorjsontest.c | 4 ++--
tests/testutilsqemuschema.c | 2 +-
tests/testutilsqemuschema.h | 2 +-
5 files changed, 6 insertions(+), 6 deletions(-)
diff --git a/tests/qemublocktest.c b/tests/qemublocktest.c
index 8057d1e2c0..0cdedb9ad4 100644
--- a/tests/qemublocktest.c
+++ b/tests/qemublocktest.c
@@ -1056,7 +1056,7 @@ mymain(void)
diskxmljsondata.qemuCaps = caps_x86_64;
imagecreatedata.qemuCaps = caps_x86_64;
-if (!(qmp_schema_x86_64 = testQEMUSchemaLoad("x86_64"))) {
+if (!(qmp_schema_x86_64 = testQEMUSchemaLoadLatest("x86_64"))) {
ret = -1;
goto cleanup;
}
diff --git a/tests/qemuhotplugtest.c b/tests/qemuhotplugtest.c
index 1843e79d77..ba3fc4d814 100644
--- a/tests/qemuhotplugtest.c
+++ b/tests/qemuhotplugtest.c
@@ -636,7 +636,7 @@ mymain(void)
if (!(driver.domainEventState = virObjectEventStateNew()))
return EXIT_FAILURE;
-if (!(qmpschema = testQEMUSchemaLoad("x86_64"))) {
+if (!(qmpschema = testQEMUSchemaLoadLatest("x86_64"))) {
VIR_TEST_VERBOSE("failed to load qapi schema\n");
return EXIT_FAILURE;
}
diff --git a/tests/qemumonitorjsontest.c b/tests/qemumonitorjsontest.c
index a50b157dbc..6d5a1d5fe2 100644
--- a/tests/qemumonitorjsontest.c
+++ b/tests/qemumonitorjsontest.c
@@ -3124,7 +3124,7 @@ mymain(void)
virEventRegisterDefaultImpl();
-if (!(qapiData.schema = testQEMUSchemaLoad("x86_64"))) {
+if (!(qapiData.schema = testQEMUSchemaLoadLatest("x86_64"))) {
VIR_TEST_VERBOSE("failed to load qapi schema");
ret = -1;
goto cleanup;
@@ -3394,7 +3394,7 @@ mymain(void)
#undef DO_TEST_QUERY_JOBS
virHashFree(qapiData.schema);
-if (!(qapiData.schema = testQEMUSchemaLoad("s390x"))) {
+if (!(qapiData.schema = testQEMUSchemaLoadLatest("s390x"))) {
VIR_TEST_VERBOSE("failed to load qapi schema for s390x");
ret = -1;
goto cleanup;
diff --git a/tests/testutilsqemuschema.c b/tests/testutilsqemuschema.c
index 898be68b0a..060a5d7054 100644
--- a/tests/testutilsqemuschema.c
+++ b/tests/testutilsqemuschema.c
@@ -661,7 +661,7 @@ testQEMUSchemaGetLatest(const char* arch)
virHashTablePtr
-testQEMUSchemaLoad(const char* arch)
+testQEMUSchemaLoadLatest(const char *arch)
{
virJSONValuePtr schema;
diff --git a/tests/testutilsqemuschema.h b/tests/testutilsqemuschema.h
index 8b6803afda..acedb77677 100644
--- a/tests/testutilsqemuschema.h
+++ b/tests/testutilsqemuschema.h
@@ -41,4 +41,4 @@ virJSONValuePtr
testQEMUSchemaGetLatest(const char* arch);
virHashTablePtr
-testQEMUSchemaLoad(const char* arch);
+testQEMUSchemaLoadLatest(const char *arch);
--
2.26.2
[PATCH 10/21] util: json: Introduce virJSONValueObjectAppendStringPrintf
Add a variant similar to virJSONValueObjectAppendString which also
formats more complex value strings with printf syntax.
Signed-off-by: Peter Krempa
---
src/libvirt_private.syms | 1 +
src/util/virjson.c | 17 +
src/util/virjson.h | 2 ++
3 files changed, 20 insertions(+)
diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
index 31813f177f..1bd02fd8ee 100644
--- a/src/libvirt_private.syms
+++ b/src/libvirt_private.syms
@@ -2361,6 +2361,7 @@ virJSONValueObjectAppendNumberLong;
virJSONValueObjectAppendNumberUint;
virJSONValueObjectAppendNumberUlong;
virJSONValueObjectAppendString;
+virJSONValueObjectAppendStringPrintf;
virJSONValueObjectCreate;
virJSONValueObjectCreateVArgs;
virJSONValueObjectDeflatten;
diff --git a/src/util/virjson.c b/src/util/virjson.c
index dc662bf8e9..6921eccb60 100644
--- a/src/util/virjson.c
+++ b/src/util/virjson.c
@@ -649,6 +649,23 @@ virJSONValueObjectAppendString(virJSONValuePtr object,
}
+int
+virJSONValueObjectAppendStringPrintf(virJSONValuePtr object,
+ const char *key,
+ const char *fmt,
+ ...)
+{
+va_list ap;
+g_autofree char *str = NULL;
+
+va_start(ap, fmt);
+str = g_strdup_vprintf(fmt, ap);
+va_end(ap);
+
+return virJSONValueObjectInsertString(object, key, str, false);
+}
+
+
int
virJSONValueObjectPrependString(virJSONValuePtr object,
const char *key,
diff --git a/src/util/virjson.h b/src/util/virjson.h
index 0894e91b59..588c977650 100644
--- a/src/util/virjson.h
+++ b/src/util/virjson.h
@@ -127,6 +127,8 @@ int virJSONValueObjectGetBoolean(virJSONValuePtr object,
const char *key, bool *
int virJSONValueObjectIsNull(virJSONValuePtr object, const char *key);
int virJSONValueObjectAppendString(virJSONValuePtr object, const char *key,
const char *value);
+int virJSONValueObjectAppendStringPrintf(virJSONValuePtr object, const char
*key, const char *fmt, ...)
+G_GNUC_PRINTF(3, 4);
int virJSONValueObjectPrependString(virJSONValuePtr object, const char *key,
const char *value);
int virJSONValueObjectAppendNumberInt(virJSONValuePtr object, const char *key,
int number);
int virJSONValueObjectAppendNumberUint(virJSONValuePtr object, const char
*key, unsigned int number);
--
2.26.2
[PATCH 21/21] qemuxml2argvtest: Add QAPI/QMP schema validation for -blockdev and -netdev
Our hotplug test cases are weak in comparison to the xml2arvtest. Use
all the the data to also valiadte the arguments for -netdev and
-blockdev against the appropriate commands of the QMP schema.
Note that currently it's done just for the _CAPS versions of tests but
commenting out a line in the test file allows to validate even cases
which don't use real capabilities.
Signed-off-by: Peter Krempa
---
tests/Makefile.am| 2 +-
tests/qemuxml2argvtest.c | 76
tests/testutilsqemu.c| 5 +++
tests/testutilsqemu.h| 1 +
4 files changed, 83 insertions(+), 1 deletion(-)
diff --git a/tests/Makefile.am b/tests/Makefile.am
index fc516376b4..f5766a7790 100644
--- a/tests/Makefile.am
+++ b/tests/Makefile.am
@@ -570,7 +570,7 @@ qemuxml2argvtest_SOURCES = \
testutils.c testutils.h \
virfilewrapper.c virfilewrapper.h \
$(NULL)
-qemuxml2argvtest_LDADD = libqemutestdriver.la \
+qemuxml2argvtest_LDADD = libqemutestdriver.la libqemumonitortestutils.la \
$(LDADDS) $(LIBXML_LIBS)
libqemuxml2argvmock_la_SOURCES = \
diff --git a/tests/qemuxml2argvtest.c b/tests/qemuxml2argvtest.c
index 5123580ee2..4f613e8f1a 100644
--- a/tests/qemuxml2argvtest.c
+++ b/tests/qemuxml2argvtest.c
@@ -18,6 +18,7 @@
# include "qemu/qemu_migration.h"
# include "qemu/qemu_process.h"
# include "qemu/qemu_slirp.h"
+# include "qemu/qemu_qapi.h"
# include "datatypes.h"
# include "conf/storage_conf.h"
# include "cpu/cpu_map.h"
@@ -26,6 +27,8 @@
# include "virmock.h"
# include "virfilewrapper.h"
# include "configmake.h"
+# include "testutilsqemuschema.h"
+# include "qemu/qemu_monitor_json.h"
# define LIBVIRT_QEMU_CAPSPRIV_H_ALLOW
# include "qemu/qemu_capspriv.h"
@@ -477,6 +480,76 @@ testCompareXMLToArgvCreateArgs(virQEMUDriverPtr drv,
}
+static int
+testCompareXMLToArgvValidateSchema(virQEMUDriverPtr drv,
+ virDomainObjPtr vm,
+ const char *migrateURI,
+ struct testQemuInfo *info,
+ unsigned int flags)
+{
+VIR_AUTOSTRINGLIST args = NULL;
+size_t nargs = 0;
+size_t i;
+g_autoptr(virHashTable) schema = NULL;
+g_autoptr(virCommand) cmd = NULL;
+
+if (info->schemafile)
+schema = testQEMUSchemaLoad(info->schemafile);
+
+/* comment out with line comment to enable schema checking for non _CAPS
tests
+if (!schema)
+schema = testQEMUSchemaLoadLatest(virArchToString(info->arch));
+// */
+
+if (!schema)
+return 0;
+
+if (!(cmd = testCompareXMLToArgvCreateArgs(drv, vm, migrateURI, info,
flags,
+ true)))
+return -1;
+
+if (virCommandGetArgList(cmd, , ) < 0)
+return -1;
+
+for (i = 0; i < nargs; i++) {
+g_auto(virBuffer) debug = VIR_BUFFER_INITIALIZER;
+g_autoptr(virJSONValue) jsonargs = NULL;
+
+if (STREQ(args[i], "-blockdev")) {
+if (!(jsonargs = virJSONValueFromString(args[i + 1])))
+return -1;
+
+if (testQEMUSchemaValidateCommand("blockdev-add", jsonargs,
+ schema, false, false, ) <
0) {
+VIR_TEST_VERBOSE("failed to validate -blockdev '%s' against
QAPI schema: %s",
+ args[i + 1], virBufferCurrentContent());
+return -1;
+}
+
+i++;
+} else if (STREQ(args[i], "-netdev")) {
+if (!(jsonargs = virJSONValueFromString(args[i + 1])))
+return -1;
+
+/* skip the validation for pre-QAPIfication cases */
+if
(virQEMUQAPISchemaPathExists("netdev_add/arg-type/type/!string", schema))
+continue;
+
+if (testQEMUSchemaValidateCommand("netdev_add", jsonargs,
+ schema, false, false, ) <
0) {
+VIR_TEST_VERBOSE("failed to validate -netdev '%s' against QAPI
schema: %s",
+ args[i + 1], virBufferCurrentContent());
+return -1;
+}
+
+i++;
+}
+}
+
+return 0;
+}
+
+
static int
testCompareXMLToArgv(const void *data)
{
@@ -578,6 +651,9 @@ testCompareXMLToArgv(const void *data)
goto cleanup;
}
+if (testCompareXMLToArgvValidateSchema(, vm, migrateURI, info,
flags) < 0)
+goto cleanup;
+
if (!(actualargv = virCommandToString(cmd, false)))
goto cleanup;
diff --git a/tests/testutilsqemu.c b/tests/testutilsqemu.c
index 9f9eb4033c..4b83b8ff25 100644
--- a/tests/testutilsqemu.c
+++ b/tests/testutilsqemu.c
@@ -768,6 +768,10 @@ testQemuInfoSetArgs(struct testQemuInfo *info,
if (stripmachinealiases)
virQEMUCapsStripMachineAliases(qemuCaps);
info->flags |= FLAG_REAL_CAPS;
+
+/* provide path to the replies file for
[PATCH 11/21] testCompareXMLToArgv: Split out preparation and command formatting
There are multiple steps of setting up the domain definition prior to
formatting the command line for the tests. Extract it to a separate
function so that it's self-contained and also will allow re-running the
command line formatting which will be necessary for QMP schema
validation tests.
Signed-off-by: Peter Krempa
---
tests/qemuxml2argvtest.c | 158 +--
1 file changed, 86 insertions(+), 72 deletions(-)
diff --git a/tests/qemuxml2argvtest.c b/tests/qemuxml2argvtest.c
index 43e76956cc..9fa1afd9d0 100644
--- a/tests/qemuxml2argvtest.c
+++ b/tests/qemuxml2argvtest.c
@@ -391,6 +391,90 @@ testCheckExclusiveFlags(int flags)
}
+static virCommandPtr
+testCompareXMLToArgvCreateArgs(virQEMUDriverPtr drv,
+ virDomainObjPtr vm,
+ const char *migrateURI,
+ struct testQemuInfo *info,
+ unsigned int flags)
+{
+size_t i;
+
+for (i = 0; i < vm->def->nhostdevs; i++) {
+virDomainHostdevDefPtr hostdev = vm->def->hostdevs[i];
+
+if (hostdev->mode == VIR_DOMAIN_HOSTDEV_MODE_SUBSYS &&
+hostdev->source.subsys.type == VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_PCI
&&
+hostdev->source.subsys.u.pci.backend ==
VIR_DOMAIN_HOSTDEV_PCI_BACKEND_DEFAULT) {
+hostdev->source.subsys.u.pci.backend =
VIR_DOMAIN_HOSTDEV_PCI_BACKEND_VFIO;
+}
+}
+
+for (i = 0; i < vm->def->nfss; i++) {
+virDomainFSDefPtr fs = vm->def->fss[i];
+char *s;
+
+if (fs->fsdriver != VIR_DOMAIN_FS_DRIVER_TYPE_VIRTIOFS ||
+QEMU_DOMAIN_FS_PRIVATE(fs)->vhostuser_fs_sock)
+continue;
+
+s = g_strdup_printf("/tmp/lib/domain--1-guest/fs%zu.vhost-fs.sock", i);
+QEMU_DOMAIN_FS_PRIVATE(fs)->vhostuser_fs_sock = s;
+}
+
+if (vm->def->vsock) {
+virDomainVsockDefPtr vsock = vm->def->vsock;
+qemuDomainVsockPrivatePtr vsockPriv =
+(qemuDomainVsockPrivatePtr)vsock->privateData;
+
+if (vsock->auto_cid == VIR_TRISTATE_BOOL_YES)
+vsock->guest_cid = 42;
+
+vsockPriv->vhostfd = 6789;
+}
+
+if (vm->def->tpm) {
+switch (vm->def->tpm->type) {
+case VIR_DOMAIN_TPM_TYPE_EMULATOR:
+VIR_FREE(vm->def->tpm->data.emulator.source.data.file.path);
+vm->def->tpm->data.emulator.source.data.file.path =
g_strdup("/dev/test");
+vm->def->tpm->data.emulator.source.type = VIR_DOMAIN_CHR_TYPE_FILE;
+break;
+case VIR_DOMAIN_TPM_TYPE_PASSTHROUGH:
+case VIR_DOMAIN_TPM_TYPE_LAST:
+break;
+ }
+}
+
+for (i = 0; i < vm->def->nvideos; i++) {
+virDomainVideoDefPtr video = vm->def->videos[i];
+
+if (video->backend == VIR_DOMAIN_VIDEO_BACKEND_TYPE_VHOSTUSER) {
+qemuDomainVideoPrivatePtr vpriv = QEMU_DOMAIN_VIDEO_PRIVATE(video);
+
+vpriv->vhost_user_fd = 1729;
+}
+}
+
+if (flags & FLAG_SLIRP_HELPER) {
+for (i = 0; i < vm->def->nnets; i++) {
+virDomainNetDefPtr net = vm->def->nets[i];
+
+if (net->type == VIR_DOMAIN_NET_TYPE_USER &&
+virQEMUCapsGet(info->qemuCaps, QEMU_CAPS_DBUS_VMSTATE)) {
+qemuSlirpPtr slirp = qemuSlirpNew();
+slirp->fd[0] = 42;
+QEMU_DOMAIN_NETWORK_PRIVATE(net)->slirp = slirp;
+}
+}
+}
+
+return qemuProcessCreatePretendCmd(drv, vm, migrateURI,
+ (flags & FLAG_FIPS), false,
+ VIR_QEMU_PROCESS_START_COLD);
+}
+
+
static int
testCompareXMLToArgv(const void *data)
{
@@ -405,7 +489,6 @@ testCompareXMLToArgv(const void *data)
virConnectPtr conn;
char *log = NULL;
virCommandPtr cmd = NULL;
-size_t i;
qemuDomainObjPrivatePtr priv = NULL;
if (info->arch != VIR_ARCH_NONE && info->arch != VIR_ARCH_X86_64)
@@ -482,77 +565,8 @@ testCompareXMLToArgv(const void *data)
VIR_FREE(log);
virResetLastError();
-for (i = 0; i < vm->def->nhostdevs; i++) {
-virDomainHostdevDefPtr hostdev = vm->def->hostdevs[i];
-
-if (hostdev->mode == VIR_DOMAIN_HOSTDEV_MODE_SUBSYS &&
-hostdev->source.subsys.type == VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_PCI
&&
-hostdev->source.subsys.u.pci.backend ==
VIR_DOMAIN_HOSTDEV_PCI_BACKEND_DEFAULT) {
-hostdev->source.subsys.u.pci.backend =
VIR_DOMAIN_HOSTDEV_PCI_BACKEND_VFIO;
-}
-}
-
-for (i = 0; i < vm->def->nfss; i++) {
-virDomainFSDefPtr fs = vm->def->fss[i];
-char *s;
-
-if (fs->fsdriver != VIR_DOMAIN_FS_DRIVER_TYPE_VIRTIOFS)
-continue;
-
-s = g_strdup_printf("/tmp/lib/domain--1-guest/fs%zu.vhost-fs.sock", i);
-QEMU_DOMAIN_FS_PRIVATE(fs)->vhostuser_fs_sock = s;
-}
-
-if (vm->def->vsock) {
-virDomainVsockDefPtr
[PATCH 13/21] util: virqemu: Introduce virQEMUBuildNetdevCommandlineFromJSON
In preparation for converting the generator of -netdev to generate JSON
which will be used to do the command line rather than the other way
around we need to introduce a convertor which properly configures
virQEMUBuildCommandLineJSON for the quirks of -netdev.
Signed-off-by: Peter Krempa
---
src/libvirt_private.syms | 1 +
src/util/virqemu.c | 22 ++
src/util/virqemu.h | 3 +++
3 files changed, 26 insertions(+)
diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
index 1bd02fd8ee..fd04fcece3 100644
--- a/src/libvirt_private.syms
+++ b/src/libvirt_private.syms
@@ -2915,6 +2915,7 @@ virQEMUBuildCommandLineJSON;
virQEMUBuildCommandLineJSONArrayBitmap;
virQEMUBuildCommandLineJSONArrayNumbered;
virQEMUBuildDriveCommandlineFromJSON;
+virQEMUBuildNetdevCommandlineFromJSON;
virQEMUBuildObjectCommandlineFromJSON;
virQEMUBuildQemuImgKeySecretOpts;
diff --git a/src/util/virqemu.c b/src/util/virqemu.c
index 549f88fcd5..0f8cab29df 100644
--- a/src/util/virqemu.c
+++ b/src/util/virqemu.c
@@ -252,6 +252,28 @@ virQEMUBuildCommandLineJSON(virJSONValuePtr value,
}
+/**
+ * virQEMUBuildNetdevCommandlineFromJSON:
+ * @props: JSON properties describing a netdev
+ *
+ * Converts @props into arguments for -netdev including all the quirks and
+ * differences between the monitor and command line syntax.
+ */
+char *
+virQEMUBuildNetdevCommandlineFromJSON(virJSONValuePtr props)
+{
+const char *type = virJSONValueObjectGetString(props, "type");
+g_auto(virBuffer) buf = VIR_BUFFER_INITIALIZER;
+
+virBufferAsprintf(, "%s,", type);
+
+if (virQEMUBuildCommandLineJSON(props, , "type", true, NULL) < 0)
+return NULL;
+
+return virBufferContentAndReset();
+}
+
+
static int
virQEMUBuildObjectCommandlineFromJSONInternal(virBufferPtr buf,
const char *type,
diff --git a/src/util/virqemu.h b/src/util/virqemu.h
index 67a5711613..22f47851df 100644
--- a/src/util/virqemu.h
+++ b/src/util/virqemu.h
@@ -49,6 +49,9 @@ int virQEMUBuildCommandLineJSON(virJSONValuePtr value,
bool onOff,
virQEMUBuildCommandLineJSONArrayFormatFunc
array);
+char *
+virQEMUBuildNetdevCommandlineFromJSON(virJSONValuePtr props);
+
int virQEMUBuildObjectCommandlineFromJSON(virBufferPtr buf,
virJSONValuePtr objprops);
--
2.26.2
[PATCH 12/21] qemuMonitorJSON(Add|Remove)Netdev: Refactor cleanup
Use automatic pointer cleanup for virJSONValuePtrs to get rid of the
cleanup label and ret variable.
Signed-off-by: Peter Krempa
---
src/qemu/qemu_monitor_json.c | 52 +++-
1 file changed, 22 insertions(+), 30 deletions(-)
diff --git a/src/qemu/qemu_monitor_json.c b/src/qemu/qemu_monitor_json.c
index 66422a8489..1224d1c9cc 100644
--- a/src/qemu/qemu_monitor_json.c
+++ b/src/qemu/qemu_monitor_json.c
@@ -3984,13 +3984,13 @@ int qemuMonitorJSONCloseFileHandle(qemuMonitorPtr mon,
}
-int qemuMonitorJSONAddNetdev(qemuMonitorPtr mon,
- const char *netdevstr)
+int
+qemuMonitorJSONAddNetdev(qemuMonitorPtr mon,
+ const char *netdevstr)
{
-int ret = -1;
-virJSONValuePtr cmd = NULL;
-virJSONValuePtr reply = NULL;
-virJSONValuePtr args = NULL;
+g_autoptr(virJSONValue) cmd = NULL;
+g_autoptr(virJSONValue) reply = NULL;
+g_autoptr(virJSONValue) args = NULL;
cmd = qemuMonitorJSONMakeCommand("netdev_add", NULL);
if (!cmd)
@@ -3998,49 +3998,41 @@ int qemuMonitorJSONAddNetdev(qemuMonitorPtr mon,
args = qemuMonitorJSONKeywordStringToJSON(netdevstr, "type");
if (!args)
-goto cleanup;
+return -1;
if (virJSONValueObjectAppend(cmd, "arguments", args) < 0)
-goto cleanup;
+return -1;
args = NULL; /* obj owns reference to args now */
if (qemuMonitorJSONCommand(mon, cmd, ) < 0)
-goto cleanup;
+return -1;
if (qemuMonitorJSONCheckError(cmd, reply) < 0)
-goto cleanup;
+return -1;
-ret = 0;
- cleanup:
-virJSONValueFree(args);
-virJSONValueFree(cmd);
-virJSONValueFree(reply);
-return ret;
+return 0;
}
-int qemuMonitorJSONRemoveNetdev(qemuMonitorPtr mon,
-const char *alias)
+int
+qemuMonitorJSONRemoveNetdev(qemuMonitorPtr mon,
+const char *alias)
{
-int ret = -1;
-virJSONValuePtr cmd = qemuMonitorJSONMakeCommand("netdev_del",
- "s:id", alias,
- NULL);
-virJSONValuePtr reply = NULL;
+g_autoptr(virJSONValue) cmd = qemuMonitorJSONMakeCommand("netdev_del",
+ "s:id", alias,
+ NULL);
+g_autoptr(virJSONValue) reply = NULL;
+
if (!cmd)
return -1;
if (qemuMonitorJSONCommand(mon, cmd, ) < 0)
-goto cleanup;
+return -1;
if (qemuMonitorJSONCheckError(cmd, reply) < 0)
-goto cleanup;
+return -1;
-ret = 0;
- cleanup:
-virJSONValueFree(cmd);
-virJSONValueFree(reply);
-return ret;
+return 0;
}
--
2.26.2
[PATCH 04/21] qemuBuildChannelsCommandLine: Extract common formatting of 'chardev'
Both active branches create the same backend chardev. Since there is no
other case, extract it before the switch so that we don't have to
duplicate it.
Signed-off-by: Peter Krempa
---
src/qemu/qemu_command.c | 32 +++-
1 file changed, 11 insertions(+), 21 deletions(-)
diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
index 06568ae585..56b4aae1dd 100644
--- a/src/qemu/qemu_command.c
+++ b/src/qemu/qemu_command.c
@@ -8521,37 +8521,27 @@ qemuBuildChannelsCommandLine(virLogManagerPtr
logManager,
for (i = 0; i < def->nchannels; i++) {
virDomainChrDefPtr channel = def->channels[i];
-char *devstr;
+g_autofree char *chardevstr = NULL;
+g_autofree char *netdevstr = NULL;
-switch ((virDomainChrChannelTargetType) channel->targetType) {
-case VIR_DOMAIN_CHR_CHANNEL_TARGET_TYPE_GUESTFWD:
-if (!(devstr = qemuBuildChrChardevStr(logManager, secManager,
+if (!(chardevstr = qemuBuildChrChardevStr(logManager, secManager,
cmd, cfg, def,
channel->source,
channel->info.alias,
qemuCaps, cdevflags)))
-return -1;
-virCommandAddArg(cmd, "-chardev");
-virCommandAddArg(cmd, devstr);
-VIR_FREE(devstr);
+return -1;
-if (qemuBuildChrDeviceStr(, def, channel, qemuCaps) < 0)
+virCommandAddArg(cmd, "-chardev");
+virCommandAddArg(cmd, chardevstr);
+
+switch ((virDomainChrChannelTargetType) channel->targetType) {
+case VIR_DOMAIN_CHR_CHANNEL_TARGET_TYPE_GUESTFWD:
+if (qemuBuildChrDeviceStr(, def, channel, qemuCaps) < 0)
return -1;
-virCommandAddArgList(cmd, "-netdev", devstr, NULL);
-VIR_FREE(devstr);
+virCommandAddArgList(cmd, "-netdev", netdevstr, NULL);
break;
case VIR_DOMAIN_CHR_CHANNEL_TARGET_TYPE_VIRTIO:
-if (!(devstr = qemuBuildChrChardevStr(logManager, secManager,
- cmd, cfg, def,
- channel->source,
- channel->info.alias,
- qemuCaps, cdevflags)))
-return -1;
-virCommandAddArg(cmd, "-chardev");
-virCommandAddArg(cmd, devstr);
-VIR_FREE(devstr);
-
if (qemuBuildChrDeviceCommandLine(cmd, def, channel, qemuCaps) < 0)
return -1;
break;
--
2.26.2
[PATCH 05/21] qemuBuildChannelChrDeviceStr: Remove formatting of properties for -netdev
The output of the function is fed as argument to '-device' command line
argument or 'device_add' monitor command except for 'guestfwd' channels
where it needs to be fed to -netdev/netdev_add. This is confusing and
error prone. Split it up since the caller needs to know which
command/option to use anyways, so the caller can call the appropriate
function without any magic.
Signed-off-by: Peter Krempa
---
src/qemu/qemu_command.c | 36
src/qemu/qemu_command.h | 3 +++
src/qemu/qemu_hotplug.c | 18 +-
3 files changed, 36 insertions(+), 21 deletions(-)
diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
index 56b4aae1dd..c20176f619 100644
--- a/src/qemu/qemu_command.c
+++ b/src/qemu/qemu_command.c
@@ -8536,7 +8536,7 @@ qemuBuildChannelsCommandLine(virLogManagerPtr logManager,
switch ((virDomainChrChannelTargetType) channel->targetType) {
case VIR_DOMAIN_CHR_CHANNEL_TARGET_TYPE_GUESTFWD:
-if (qemuBuildChrDeviceStr(, def, channel, qemuCaps) < 0)
+if (!(netdevstr = qemuBuildChannelGuestfwdNetdevProps(channel)))
return -1;
virCommandAddArgList(cmd, "-netdev", netdevstr, NULL);
break;
@@ -9804,36 +9804,40 @@ qemuBuildParallelChrDeviceStr(char **deviceStr,
return 0;
}
-static int
-qemuBuildChannelChrDeviceStr(char **deviceStr,
- const virDomainDef *def,
- virDomainChrDefPtr chr)
+
+char *
+qemuBuildChannelGuestfwdNetdevProps(virDomainChrDefPtr chr)
{
-int ret = -1;
g_autofree char *addr = NULL;
int port;
-switch ((virDomainChrChannelTargetType)chr->targetType) {
-case VIR_DOMAIN_CHR_CHANNEL_TARGET_TYPE_GUESTFWD:
+if (!(addr = virSocketAddrFormat(chr->target.addr)))
+return NULL;
-addr = virSocketAddrFormat(chr->target.addr);
-if (!addr)
-return ret;
-port = virSocketAddrGetPort(chr->target.addr);
+port = virSocketAddrGetPort(chr->target.addr);
+
+return g_strdup_printf("user,guestfwd=tcp:%s:%i-chardev:char%s,id=%s",
+ addr, port, chr->info.alias, chr->info.alias);
+}
-*deviceStr =
g_strdup_printf("user,guestfwd=tcp:%s:%i-chardev:char%s,id=%s",
- addr, port, chr->info.alias,
chr->info.alias);
-break;
+static int
+qemuBuildChannelChrDeviceStr(char **deviceStr,
+ const virDomainDef *def,
+ virDomainChrDefPtr chr)
+{
+switch ((virDomainChrChannelTargetType)chr->targetType) {
case VIR_DOMAIN_CHR_CHANNEL_TARGET_TYPE_VIRTIO:
if (!(*deviceStr = qemuBuildVirtioSerialPortDevStr(def, chr)))
return -1;
break;
+case VIR_DOMAIN_CHR_CHANNEL_TARGET_TYPE_GUESTFWD:
+/* guestfwd is as a netdev handled separately */
case VIR_DOMAIN_CHR_CHANNEL_TARGET_TYPE_XEN:
case VIR_DOMAIN_CHR_CHANNEL_TARGET_TYPE_NONE:
case VIR_DOMAIN_CHR_CHANNEL_TARGET_TYPE_LAST:
-return ret;
+return -1;
}
return 0;
diff --git a/src/qemu/qemu_command.h b/src/qemu/qemu_command.h
index 53e05777e7..7665b68548 100644
--- a/src/qemu/qemu_command.h
+++ b/src/qemu/qemu_command.h
@@ -86,6 +86,9 @@ qemuBuildChrDeviceStr(char **deviceStr,
virDomainChrDefPtr chr,
virQEMUCapsPtr qemuCaps);
+char *
+qemuBuildChannelGuestfwdNetdevProps(virDomainChrDefPtr chr);
+
char *qemuBuildHostNetStr(virDomainNetDefPtr net,
char **tapfd,
size_t tapfdSize,
diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c
index ab5a7aef84..2976ba7647 100644
--- a/src/qemu/qemu_hotplug.c
+++ b/src/qemu/qemu_hotplug.c
@@ -2108,6 +2108,7 @@ int qemuDomainAttachChrDevice(virQEMUDriverPtr driver,
virErrorPtr orig_err;
virDomainDefPtr vmdef = vm->def;
g_autofree char *devstr = NULL;
+g_autofree char *netdevstr = NULL;
virDomainChrSourceDefPtr dev = chr->source;
g_autofree char *charAlias = NULL;
bool chardevAttached = false;
@@ -2146,8 +2147,13 @@ int qemuDomainAttachChrDevice(virQEMUDriverPtr driver,
goto cleanup;
teardowncgroup = true;
-if (qemuBuildChrDeviceStr(, vmdef, chr, priv->qemuCaps) < 0)
-goto cleanup;
+if (guestfwd) {
+if (!(netdevstr = qemuBuildChannelGuestfwdNetdevProps(chr)))
+goto cleanup;
+} else {
+if (qemuBuildChrDeviceStr(, vmdef, chr, priv->qemuCaps) < 0)
+goto cleanup;
+}
if (!(charAlias = qemuAliasChardevFromDevAlias(chr->info.alias)))
goto cleanup;
@@ -2166,11 +2172,13 @@ int qemuDomainAttachChrDevice(virQEMUDriverPtr driver,
goto exit_monitor;
chardevAttached = true;
-if (guestfwd) {
-if (qemuMonitorAddNetdev(priv->mon, devstr,
+if (netdevstr) {
+if (qemuMonitorAddNetdev(priv->mon, netdevstr,
[PATCH 09/21] virCommand: Introduce virCommandGetArgList
The helper returns a list of arguments of a virCommand. This will be
useful in tests where we'll inspect certain already formatted arguments.
Signed-off-by: Peter Krempa
---
src/libvirt_private.syms | 1 +
src/util/vircommand.c| 23 +++
src/util/vircommand.h| 1 +
3 files changed, 25 insertions(+)
diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
index 935ef7303b..31813f177f 100644
--- a/src/libvirt_private.syms
+++ b/src/libvirt_private.syms
@@ -1841,6 +1841,7 @@ virCommandDaemonize;
virCommandDoAsyncIO;
virCommandExec;
virCommandFree;
+virCommandGetArgList;
virCommandGetGID;
virCommandGetUID;
virCommandHandshakeNotify;
diff --git a/src/util/vircommand.c b/src/util/vircommand.c
index 20f196104f..aae0ddb730 100644
--- a/src/util/vircommand.c
+++ b/src/util/vircommand.c
@@ -2132,6 +2132,29 @@ virCommandToString(virCommandPtr cmd, bool linebreaks)
}
+int
+virCommandGetArgList(virCommandPtr cmd,
+ char ***args,
+ size_t *nargs)
+{
+size_t i;
+
+if (cmd->has_error) {
+virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
+ _("invalid use of command API"));
+return -1;
+}
+
+*args = g_new0(char *, cmd->nargs);
+*nargs = cmd->nargs - 1;
+
+for (i = 1; i < cmd->nargs; i++)
+(*args)[i - 1] = g_strdup(cmd->args[i]);
+
+return 0;
+}
+
+
#ifndef WIN32
/*
* Manage input and output to the child process.
diff --git a/src/util/vircommand.h b/src/util/vircommand.h
index e2be5bcf1c..ff8a785dbe 100644
--- a/src/util/vircommand.h
+++ b/src/util/vircommand.h
@@ -170,6 +170,7 @@ void virCommandWriteArgLog(virCommandPtr cmd,
int logfd);
char *virCommandToString(virCommandPtr cmd, bool linebreaks)
G_GNUC_WARN_UNUSED_RESULT;
+int virCommandGetArgList(virCommandPtr cmd, char ***args, size_t *nargs);
int virCommandExec(virCommandPtr cmd, gid_t *groups, int ngroups)
G_GNUC_WARN_UNUSED_RESULT;
--
2.26.2
[PATCH 03/21] qemuBuildChannelsCommandLine: Use typecasted switch for channel type
Cover all cases of the enum.
Signed-off-by: Peter Krempa
---
src/qemu/qemu_command.c | 7 ++-
1 file changed, 6 insertions(+), 1 deletion(-)
diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
index bfe70ed228..06568ae585 100644
--- a/src/qemu/qemu_command.c
+++ b/src/qemu/qemu_command.c
@@ -8523,7 +8523,7 @@ qemuBuildChannelsCommandLine(virLogManagerPtr logManager,
virDomainChrDefPtr channel = def->channels[i];
char *devstr;
-switch (channel->targetType) {
+switch ((virDomainChrChannelTargetType) channel->targetType) {
case VIR_DOMAIN_CHR_CHANNEL_TARGET_TYPE_GUESTFWD:
if (!(devstr = qemuBuildChrChardevStr(logManager, secManager,
cmd, cfg, def,
@@ -8555,6 +8555,11 @@ qemuBuildChannelsCommandLine(virLogManagerPtr logManager,
if (qemuBuildChrDeviceCommandLine(cmd, def, channel, qemuCaps) < 0)
return -1;
break;
+
+case VIR_DOMAIN_CHR_CHANNEL_TARGET_TYPE_XEN:
+case VIR_DOMAIN_CHR_CHANNEL_TARGET_TYPE_NONE:
+case VIR_DOMAIN_CHR_CHANNEL_TARGET_TYPE_LAST:
+return -1;
}
}
--
2.26.2
[PATCH 07/21] virQEMUBuildCommandLineJSON: Allow skipping certain keys
Allow reusing this for formatting of netdev_add arguments into -netdev.
We need to be able to skip the 'type' property as it's used without the
prefix.
Add infrastructure which allows skipping a certainly named property.
Signed-off-by: Peter Krempa
---
src/util/virqemu.c | 30 +-
src/util/virqemu.h | 10 +++---
tests/qemucommandutiltest.c | 2 +-
3 files changed, 29 insertions(+), 13 deletions(-)
diff --git a/src/util/virqemu.c b/src/util/virqemu.c
index 78a9e0480b..0e6fa412bc 100644
--- a/src/util/virqemu.c
+++ b/src/util/virqemu.c
@@ -36,6 +36,7 @@ VIR_LOG_INIT("util.qemu");
struct virQEMUCommandLineJSONIteratorData {
const char *prefix;
virBufferPtr buf;
+const char *skipKey;
virQEMUBuildCommandLineJSONArrayFormatFunc arrayFunc;
};
@@ -44,6 +45,7 @@ static int
virQEMUBuildCommandLineJSONRecurse(const char *key,
virJSONValuePtr value,
virBufferPtr buf,
+ const char *skipKey,
virQEMUBuildCommandLineJSONArrayFormatFunc
arrayFunc,
bool nested);
@@ -52,7 +54,8 @@ virQEMUBuildCommandLineJSONRecurse(const char *key,
int
virQEMUBuildCommandLineJSONArrayBitmap(const char *key,
virJSONValuePtr array,
- virBufferPtr buf)
+ virBufferPtr buf,
+ const char *skipKey G_GNUC_UNUSED)
{
ssize_t pos = -1;
ssize_t end;
@@ -80,7 +83,8 @@ virQEMUBuildCommandLineJSONArrayBitmap(const char *key,
int
virQEMUBuildCommandLineJSONArrayNumbered(const char *key,
virJSONValuePtr array,
- virBufferPtr buf)
+ virBufferPtr buf,
+ const char *skipKey)
{
virJSONValuePtr member;
size_t i;
@@ -91,7 +95,7 @@ virQEMUBuildCommandLineJSONArrayNumbered(const char *key,
prefix = g_strdup_printf("%s.%zu", key, i);
-if (virQEMUBuildCommandLineJSONRecurse(prefix, member, buf,
+if (virQEMUBuildCommandLineJSONRecurse(prefix, member, buf, skipKey,
virQEMUBuildCommandLineJSONArrayNumbered,
true) < 0)
return 0;
@@ -109,15 +113,20 @@ virQEMUBuildCommandLineJSONIterate(const char *key,
{
struct virQEMUCommandLineJSONIteratorData *data = opaque;
+if (STREQ_NULLABLE(key, data->skipKey))
+return 0;
+
if (data->prefix) {
g_autofree char *tmpkey = NULL;
tmpkey = g_strdup_printf("%s.%s", data->prefix, key);
return virQEMUBuildCommandLineJSONRecurse(tmpkey, value, data->buf,
+ data->skipKey,
data->arrayFunc, false);
} else {
return virQEMUBuildCommandLineJSONRecurse(key, value, data->buf,
+ data->skipKey,
data->arrayFunc, false);
}
}
@@ -127,10 +136,11 @@ static int
virQEMUBuildCommandLineJSONRecurse(const char *key,
virJSONValuePtr value,
virBufferPtr buf,
+ const char *skipKey,
virQEMUBuildCommandLineJSONArrayFormatFunc
arrayFunc,
bool nested)
{
-struct virQEMUCommandLineJSONIteratorData data = { key, buf, arrayFunc };
+struct virQEMUCommandLineJSONIteratorData data = { key, buf, skipKey,
arrayFunc };
virJSONType type = virJSONValueGetType(value);
virJSONValuePtr elem;
bool tmp;
@@ -170,14 +180,14 @@ virQEMUBuildCommandLineJSONRecurse(const char *key,
return -1;
}
-if (!arrayFunc || arrayFunc(key, value, buf) < 0) {
+if (!arrayFunc || arrayFunc(key, value, buf, skipKey) < 0) {
/* fallback, treat the array as a non-bitmap, adding the key
* for each member */
for (i = 0; i < virJSONValueArraySize(value); i++) {
elem = virJSONValueArrayGet((virJSONValuePtr)value, i);
/* recurse to avoid duplicating code */
-if (virQEMUBuildCommandLineJSONRecurse(key, elem, buf,
+if (virQEMUBuildCommandLineJSONRecurse(key, elem, buf, skipKey,
arrayFunc, true) < 0)
return -1;
}
@@ -205,6 +215,7 @@ virQEMUBuildCommandLineJSONRecurse(const char *key,
* virQEMUBuildCommandLineJSON:
* @value: json object containing the value
* @buf: otuput buffer
+ *
[PATCH 02/21] qemuMonitorJSONParseKeywords: remove constant argument
There's just one caller that always passes '1'.
Signed-off-by: Peter Krempa
---
src/qemu/qemu_monitor_json.c | 17 +
1 file changed, 5 insertions(+), 12 deletions(-)
diff --git a/src/qemu/qemu_monitor_json.c b/src/qemu/qemu_monitor_json.c
index 505b31a78a..66422a8489 100644
--- a/src/qemu/qemu_monitor_json.c
+++ b/src/qemu/qemu_monitor_json.c
@@ -604,16 +604,15 @@ qemuMonitorJSONParseKeywordsFree(int nkeywords,
/*
* Takes a string containing a set of key=value,key=value,key...
* parameters and splits them up, returning two arrays with
- * the individual keys and values. If allowEmptyValue is nonzero,
- * the "=value" part is optional and if a key with no value is found,
+ * the individual keys and values.
+ * The "=value" part is optional and if a key with no value is found,
* NULL is be placed into corresponding place in retvalues.
*/
static int
qemuMonitorJSONParseKeywords(const char *str,
char ***retkeywords,
char ***retvalues,
- int *retnkeywords,
- int allowEmptyValue)
+ int *retnkeywords)
{
int keywordCount = 0;
int keywordAlloc = 0;
@@ -645,14 +644,8 @@ qemuMonitorJSONParseKeywords(const char *str,
if (!(separator = strchr(start, '=')))
separator = end;
-if (separator >= endmark) {
-if (!allowEmptyValue) {
-virReportError(VIR_ERR_INTERNAL_ERROR,
- _("malformed keyword arguments in '%s'"), str);
-goto error;
-}
+if (separator >= endmark)
separator = endmark;
-}
keyword = g_strndup(start, separator - start);
@@ -708,7 +701,7 @@ qemuMonitorJSONKeywordStringToJSON(const char *str, const
char *firstkeyword)
int nkeywords = 0;
size_t i;
-if (qemuMonitorJSONParseKeywords(str, , , , 1) <
0)
+if (qemuMonitorJSONParseKeywords(str, , , ) < 0)
goto error;
for (i = 0; i < nkeywords; i++) {
--
2.26.2
[PATCH 06/21] qemuBuildHostNetStr: Stop using 'ipv6-net' convenience argument
In qemu the argument of 'ipv6-net' is split up into 'ipv6-prefix' and
'ipv6-prefixlen'. Additionally now that 'netdev_add' was qapified, only
the real properties are allowed. Switch to using them explicitly.
Signed-off-by: Peter Krempa
---
src/qemu/qemu_command.c | 20 ++--
tests/qemuxml2argvdata/net-user-addr.args | 3 ++-
2 files changed, 12 insertions(+), 11 deletions(-)
diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
index c20176f619..2850953bd0 100644
--- a/src/qemu/qemu_command.c
+++ b/src/qemu/qemu_command.c
@@ -3645,20 +3645,20 @@ qemuBuildHostNetStr(virDomainNetDefPtr net,
for (i = 0; i < net->guestIP.nips; i++) {
const virNetDevIPAddr *ip = net->guestIP.ips[i];
g_autofree char *addr = NULL;
-const char *prefix = "";
if (!(addr = virSocketAddrFormat(>address)))
return NULL;
-if (VIR_SOCKET_ADDR_IS_FAMILY(>address, AF_INET))
-prefix = "net=";
-if (VIR_SOCKET_ADDR_IS_FAMILY(>address, AF_INET6))
-prefix = "ipv6-net=";
-
-virBufferAsprintf(, "%s%s", prefix, addr);
-if (ip->prefix)
-virBufferAsprintf(, "/%u", ip->prefix);
-virBufferAddChar(, ',');
+if (VIR_SOCKET_ADDR_IS_FAMILY(>address, AF_INET)) {
+virBufferAsprintf(, "net=%s", addr);
+if (ip->prefix)
+virBufferAsprintf(, "/%u", ip->prefix);
+virBufferAddChar(, ',');
+} else if (VIR_SOCKET_ADDR_IS_FAMILY(>address, AF_INET6)) {
+virBufferAsprintf(, "ipv6-prefix=%s,", addr);
+if (ip->prefix)
+virBufferAsprintf(, "ipv6-prefixlen=%u,",
ip->prefix);
+}
}
}
break;
diff --git a/tests/qemuxml2argvdata/net-user-addr.args
b/tests/qemuxml2argvdata/net-user-addr.args
index 6cc82d9e62..5f1de305e0 100644
--- a/tests/qemuxml2argvdata/net-user-addr.args
+++ b/tests/qemuxml2argvdata/net-user-addr.args
@@ -27,6 +27,7 @@ server,nowait \
-usb \
-drive file=/dev/HostVG/QEMUGuest1,format=raw,if=none,id=drive-ide0-0-0 \
-device ide-hd,bus=ide.0,unit=0,drive=drive-ide0-0-0,id=ide0-0-0,bootindex=1 \
--netdev user,net=172.17.2.0/24,ipv6-net=2001:db8:ac10:fd01::/64,id=hostnet0 \
+-netdev user,net=172.17.2.0/24,ipv6-prefix=2001:db8:ac10:fd01::,\
+ipv6-prefixlen=64,id=hostnet0 \
-device rtl8139,netdev=hostnet0,id=net0,mac=00:11:22:33:44:55,bus=pci.0,\
addr=0x3
--
2.26.2
[PATCH 01/21] qemu: domain: Forbid unsupported 'tftp' protocol and handle tests
'tftp' storage protocol was supported by qemu until 2.7.0. Add an
interlock when blockdev is used and drop the test case for it as it's
IMO not worth adding another test file just for that.
Signed-off-by: Peter Krempa
---
src/qemu/qemu_domain.c | 10 ++
tests/qemuxml2argvdata/disk-cdrom-network.args | 3 ---
.../disk-cdrom-network.x86_64-2.12.0.args | 3 ---
.../disk-cdrom-network.x86_64-latest.args | 17 ++---
tests/qemuxml2argvdata/disk-cdrom-network.xml | 9 -
5 files changed, 16 insertions(+), 26 deletions(-)
diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c
index d0528dbfe0..d5e3d1a3cc 100644
--- a/src/qemu/qemu_domain.c
+++ b/src/qemu/qemu_domain.c
@@ -5319,6 +5319,16 @@ qemuDomainValidateStorageSource(virStorageSourcePtr src,
return -1;
}
+/* TFTP protocol was not supported for some time, lock it out at least with
+ * -blockdev */
+if (actualType == VIR_STORAGE_TYPE_NETWORK &&
+src->protocol == VIR_STORAGE_NET_PROTOCOL_TFTP &&
+blockdev) {
+virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
+ _("'tftp' protocol is not supported with this QEMU
binary"));
+return -1;
+}
+
return 0;
}
diff --git a/tests/qemuxml2argvdata/disk-cdrom-network.args
b/tests/qemuxml2argvdata/disk-cdrom-network.args
index 81ff324a0f..794fdecdcb 100644
--- a/tests/qemuxml2argvdata/disk-cdrom-network.args
+++ b/tests/qemuxml2argvdata/disk-cdrom-network.args
@@ -33,7 +33,4 @@ id=drive-ide0-0-1,readonly=on \
-drive 'file=https://host.name:443/url/path/file.iso?test=val,format=raw,\
if=none,id=drive-ide0-1-0,readonly=on' \
-device ide-cd,bus=ide.1,unit=0,drive=drive-ide0-1-0,id=ide0-1-0 \
--drive file=tftp://host.name:69/url/path/file.iso,format=raw,if=none,\
-id=drive-ide0-1-1,readonly=on \
--device ide-cd,bus=ide.1,unit=1,drive=drive-ide0-1-1,id=ide0-1-1 \
-device virtio-balloon-pci,id=balloon0,bus=pci.0,addr=0x3
diff --git a/tests/qemuxml2argvdata/disk-cdrom-network.x86_64-2.12.0.args
b/tests/qemuxml2argvdata/disk-cdrom-network.x86_64-2.12.0.args
index 81f6b400aa..fa5c0ba087 100644
--- a/tests/qemuxml2argvdata/disk-cdrom-network.x86_64-2.12.0.args
+++ b/tests/qemuxml2argvdata/disk-cdrom-network.x86_64-2.12.0.args
@@ -35,9 +35,6 @@ id=drive-ide0-0-1,readonly=on \
-drive 'file=https://host.name:443/url/path/file.iso?test=val,format=raw,\
if=none,id=drive-ide0-1-0,readonly=on' \
-device ide-cd,bus=ide.1,unit=0,drive=drive-ide0-1-0,id=ide0-1-0 \
--drive file=tftp://host.name:69/url/path/file.iso,format=raw,if=none,\
-id=drive-ide0-1-1,readonly=on \
--device ide-cd,bus=ide.1,unit=1,drive=drive-ide0-1-1,id=ide0-1-1 \
-device virtio-balloon-pci,id=balloon0,bus=pci.0,addr=0x2 \
-sandbox on,obsolete=deny,elevateprivileges=deny,spawn=deny,\
resourcecontrol=deny \
diff --git a/tests/qemuxml2argvdata/disk-cdrom-network.x86_64-latest.args
b/tests/qemuxml2argvdata/disk-cdrom-network.x86_64-latest.args
index 2515b256d0..1a102949df 100644
--- a/tests/qemuxml2argvdata/disk-cdrom-network.x86_64-latest.args
+++ b/tests/qemuxml2argvdata/disk-cdrom-network.x86_64-latest.args
@@ -28,26 +28,21 @@ file=/tmp/lib/domain--1-QEMUGuest1/master-key.aes \
-boot strict=on \
-device piix3-usb-uhci,id=usb,bus=pci.0,addr=0x1.0x2 \
-blockdev '{"driver":"ftp","url":"ftp://host.name:21/url/path/file.iso",\
-"node-name":"libvirt-4-storage","auto-read-only":true,"discard":"unmap"}' \
--blockdev '{"node-name":"libvirt-4-format","read-only":true,"driver":"raw",\
-"file":"libvirt-4-storage"}' \
--device ide-cd,bus=ide.0,unit=0,drive=libvirt-4-format,id=ide0-0-0,bootindex=1
\
--blockdev '{"driver":"ftps","url":"ftps://host.name:990/url/path/file.iso",\
"node-name":"libvirt-3-storage","auto-read-only":true,"discard":"unmap"}' \
-blockdev '{"node-name":"libvirt-3-format","read-only":true,"driver":"raw",\
"file":"libvirt-3-storage"}' \
--device ide-cd,bus=ide.0,unit=1,drive=libvirt-3-format,id=ide0-0-1 \
--blockdev '{"driver":"https",\
-"url":"https://host.name:443/url/path/file.iso?test=val",\
+-device ide-cd,bus=ide.0,unit=0,drive=libvirt-3-format,id=ide0-0-0,bootindex=1
\
+-blockdev '{"driver":"ftps","url":"ftps://host.name:990/url/path/file.iso",\
"node-name":"libvirt-2-storage","auto-read-only":true,"discard":"unmap"}' \
-blockdev '{"node-name":"libvirt-2-format","read-only":true,"driver":"raw",\
"file":"libvirt-2-storage"}' \
--device ide-cd,bus=ide.1,unit=0,drive=libvirt-2-format,id=ide0-1-0 \
--blockdev '{"driver":"tftp","url":"tftp://host.name:69/url/path/file.iso",\
+-device ide-cd,bus=ide.0,unit=1,drive=libvirt-2-format,id=ide0-0-1 \
+-blockdev '{"driver":"https",\
+"url":"https://host.name:443/url/path/file.iso?test=val",\
"node-name":"libvirt-1-storage","auto-read-only":true,"discard":"unmap"}' \
-blockdev '{"node-name":"libvirt-1-format","read-only":true,"driver":"raw",\
"file":"libvirt-1-storage"}' \
--device
[PATCH 08/21] virQEMUBuildCommandLineJSON: Add possibility for using 'on/off' instead of 'yes/no'
In some cases we use 'on/off' for command line arguments. Add a switch
which will select the prefred spelling for a specific usage.
Signed-off-by: Peter Krempa
---
src/util/virqemu.c | 44 -
src/util/virqemu.h | 10 ++---
tests/qemucommandutiltest.c | 2 +-
3 files changed, 37 insertions(+), 19 deletions(-)
diff --git a/src/util/virqemu.c b/src/util/virqemu.c
index 0e6fa412bc..549f88fcd5 100644
--- a/src/util/virqemu.c
+++ b/src/util/virqemu.c
@@ -37,6 +37,7 @@ struct virQEMUCommandLineJSONIteratorData {
const char *prefix;
virBufferPtr buf;
const char *skipKey;
+bool onOff;
virQEMUBuildCommandLineJSONArrayFormatFunc arrayFunc;
};
@@ -46,6 +47,7 @@ virQEMUBuildCommandLineJSONRecurse(const char *key,
virJSONValuePtr value,
virBufferPtr buf,
const char *skipKey,
+ bool onOff,
virQEMUBuildCommandLineJSONArrayFormatFunc
arrayFunc,
bool nested);
@@ -55,7 +57,8 @@ int
virQEMUBuildCommandLineJSONArrayBitmap(const char *key,
virJSONValuePtr array,
virBufferPtr buf,
- const char *skipKey G_GNUC_UNUSED)
+ const char *skipKey G_GNUC_UNUSED,
+ bool onOff G_GNUC_UNUSED)
{
ssize_t pos = -1;
ssize_t end;
@@ -84,7 +87,8 @@ int
virQEMUBuildCommandLineJSONArrayNumbered(const char *key,
virJSONValuePtr array,
virBufferPtr buf,
- const char *skipKey)
+ const char *skipKey,
+ bool onOff)
{
virJSONValuePtr member;
size_t i;
@@ -95,7 +99,7 @@ virQEMUBuildCommandLineJSONArrayNumbered(const char *key,
prefix = g_strdup_printf("%s.%zu", key, i);
-if (virQEMUBuildCommandLineJSONRecurse(prefix, member, buf, skipKey,
+if (virQEMUBuildCommandLineJSONRecurse(prefix, member, buf, skipKey,
onOff,
virQEMUBuildCommandLineJSONArrayNumbered,
true) < 0)
return 0;
@@ -122,11 +126,11 @@ virQEMUBuildCommandLineJSONIterate(const char *key,
tmpkey = g_strdup_printf("%s.%s", data->prefix, key);
return virQEMUBuildCommandLineJSONRecurse(tmpkey, value, data->buf,
- data->skipKey,
+ data->skipKey, data->onOff,
data->arrayFunc, false);
} else {
return virQEMUBuildCommandLineJSONRecurse(key, value, data->buf,
- data->skipKey,
+ data->skipKey, data->onOff,
data->arrayFunc, false);
}
}
@@ -137,10 +141,11 @@ virQEMUBuildCommandLineJSONRecurse(const char *key,
virJSONValuePtr value,
virBufferPtr buf,
const char *skipKey,
+ bool onOff,
virQEMUBuildCommandLineJSONArrayFormatFunc
arrayFunc,
bool nested)
{
-struct virQEMUCommandLineJSONIteratorData data = { key, buf, skipKey,
arrayFunc };
+struct virQEMUCommandLineJSONIteratorData data = { key, buf, skipKey,
onOff, arrayFunc };
virJSONType type = virJSONValueGetType(value);
virJSONValuePtr elem;
bool tmp;
@@ -165,10 +170,17 @@ virQEMUBuildCommandLineJSONRecurse(const char *key,
case VIR_JSON_TYPE_BOOLEAN:
virJSONValueGetBoolean(value, );
-if (tmp)
-virBufferAsprintf(buf, "%s=yes,", key);
-else
-virBufferAsprintf(buf, "%s=no,", key);
+if (onOff) {
+if (tmp)
+virBufferAsprintf(buf, "%s=on,", key);
+else
+virBufferAsprintf(buf, "%s=off,", key);
+} else {
+if (tmp)
+virBufferAsprintf(buf, "%s=yes,", key);
+else
+virBufferAsprintf(buf, "%s=no,", key);
+}
break;
@@ -180,7 +192,7 @@ virQEMUBuildCommandLineJSONRecurse(const char *key,
return -1;
}
-if (!arrayFunc || arrayFunc(key, value, buf, skipKey) < 0) {
+if (!arrayFunc || arrayFunc(key, value, buf, skipKey, onOff) < 0) {
/* fallback, treat the array as a non-bitmap, adding the key
* for
[PATCH 00/21] Fix compatibility with netdev-add in qemu-5.0
This is based on top of tests: qemu: Detect deprecation in the QMP schema (deprecation part 1) https://www.redhat.com/archives/libvir-list/2020-April/msg01444.html Both can be fetched at: git fetch https://gitlab.com/pipo.sk/libvirt.git netdev_add-validate QEMU-5.0 released a fully QAPIfied 'netdev_add' command. Since we didn't format the arguments in a way qemu now thinks we should do them network device hotplug broke. Fix it by modifying our internals slightly and add some testing. This series also adds schema testing of -blockdev arguments and prepares for a simple addition of schema testing for arguments of -device and -object if qemu ever finishes the qapification of those. Theoretically we can even do it pre-emtptively but it's almost guaranteed to fail at least for -device. I just hope that qemu let's us know in advance as -device will be harder. Note that if the loading of the schema in the tests I've added will be deemed to be slow I'll add caching. Peter Krempa (21): qemu: domain: Forbid unsupported 'tftp' protocol and handle tests qemuMonitorJSONParseKeywords: remove constant argument qemuBuildChannelsCommandLine: Use typecasted switch for channel type qemuBuildChannelsCommandLine: Extract common formatting of 'chardev' qemuBuildChannelChrDeviceStr: Remove formatting of properties for -netdev qemuBuildHostNetStr: Stop using 'ipv6-net' convenience argument virQEMUBuildCommandLineJSON: Allow skipping certain keys virQEMUBuildCommandLineJSON: Add possibility for using 'on/off' instead of 'yes/no' virCommand: Introduce virCommandGetArgList util: json: Introduce virJSONValueObjectAppendStringPrintf testCompareXMLToArgv: Split out preparation and command formatting qemuMonitorJSON(Add|Remove)Netdev: Refactor cleanup util: virqemu: Introduce virQEMUBuildNetdevCommandlineFromJSON qemu: command: Generate -netdev command line via JSON->cmdline conversion virQEMUBuildNetdevCommandlineFromJSON: Prepare for quirky 'guestfwd' qemuBuildChannelGuestfwdNetdevProps: Convert to generating JSON props qemuMonitorAddNetdev: Convert to the native JSON props object qemu: Prepare for testing of 'netdev_add' props via qemuxml2argvtest testQEMUSchemaLoad: Rename to testQEMUSchemaLoadLatest testutilsqemuschema: Allow loading non-latest schema qemuxml2argvtest: Add QAPI/QMP schema validation for -blockdev and -netdev src/libvirt_private.syms | 3 + src/qemu/qemu_command.c | 289 +++--- src/qemu/qemu_command.h | 22 +- src/qemu/qemu_domain.c| 10 + src/qemu/qemu_driver.c| 2 +- src/qemu/qemu_hotplug.c | 31 +- src/qemu/qemu_monitor.c | 8 +- src/qemu/qemu_monitor.h | 2 +- src/qemu/qemu_monitor_json.c | 76 ++--- src/qemu/qemu_monitor_json.h | 2 +- src/qemu/qemu_process.c | 11 +- src/qemu/qemu_process.h | 1 + src/util/vircommand.c | 23 ++ src/util/vircommand.h | 1 + src/util/virjson.c| 17 ++ src/util/virjson.h| 2 + src/util/virqemu.c| 119 +++- src/util/virqemu.h| 18 +- tests/Makefile.am | 2 +- tests/qemublocktest.c | 2 +- tests/qemucommandutiltest.c | 2 +- tests/qemuhotplugtest.c | 2 +- tests/qemumonitorjsontest.c | 6 +- .../qemuxml2argvdata/disk-cdrom-network.args | 3 - .../disk-cdrom-network.x86_64-2.12.0.args | 3 - .../disk-cdrom-network.x86_64-latest.args | 17 +- tests/qemuxml2argvdata/disk-cdrom-network.xml | 9 - tests/qemuxml2argvdata/net-user-addr.args | 3 +- tests/qemuxml2argvtest.c | 236 +- tests/testutilsqemu.c | 5 + tests/testutilsqemu.h | 1 + tests/testutilsqemuschema.c | 66 ++-- tests/testutilsqemuschema.h | 5 +- 33 files changed, 667 insertions(+), 332 deletions(-) -- 2.26.2
Re: [PATCH v2 3/3] qemuBuildNumaArgStr: Use modern -numa memdev= if old -numa mem= is unsupported
On a Thursday in 2020, Michal Privoznik wrote:
In previous commit we started tracking whether QEMU supports
'-numa mem='. This is tied to the machine type because migration
from '-numa mem=' to '-numa memdev' is impossible (or vice
versa). But since it's tied to a machine type (where migration
from one to another is also unsupported) we can allow QEMU to get
rid of the deprecated command line.
(At least) this commit should have the bugzilla link.
Signed-off-by: Michal Privoznik
---
src/qemu/qemu_command.c | 9 +
1 file changed, 9 insertions(+)
diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
index cf6c48d233..19c45cb4e3 100644
--- a/src/qemu/qemu_command.c
+++ b/src/qemu/qemu_command.c
@@ -7038,6 +7038,11 @@ qemuBuildNumaArgStr(virQEMUDriverConfigPtr cfg,
if (!virDomainNumatuneNodesetIsAvailable(def->numa, priv->autoNodeset))
goto cleanup;
+if (!virQEMUCapsGetMachineNumaMemSupported(qemuCaps,
+ def->virtType,
+ def->os.machine))
+needBackend = true;
+
if (VIR_ALLOC_N(nodeBackends, ncells) < 0)
goto cleanup;
@@ -7055,6 +7060,10 @@ qemuBuildNumaArgStr(virQEMUDriverConfigPtr cfg,
if (rc == 0)
needBackend = true;
}
+} else if (needBackend) {
+virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
+ _("numa not supported"));
s/numa/NUMA/
Also, can we be more helpful, e.g.
"NUMA without specified memory backing is not supported with this QEMU
binary"
+goto cleanup;
}
Either way:
Reviewed-by: Ján Tomko
Jano
signature.asc
Description: PGP signature
Re: [PATCH 35/40] libxl: convert libxlMigrationDstArgs to GObject
On Wed, May 13, 2020 at 01:57:19PM +0200, Rafael Fonseca wrote:
> Signed-off-by: Rafael Fonseca
> ---
> src/libxl/libxl_migration.c | 65 +
> 1 file changed, 37 insertions(+), 28 deletions(-)
>
> diff --git a/src/libxl/libxl_migration.c b/src/libxl/libxl_migration.c
> index 50225855ae..678d850cf6 100644
> --- a/src/libxl/libxl_migration.c
> +++ b/src/libxl/libxl_migration.c
This doesn't seem to have even been compile tested.
Since we're using GitLab now, if you just fork the libvirt repo
and push a branch to your gitlab fork, it will run CI which will
check for these mistakes.
diff --git a/src/libxl/libxl_migration.c b/src/libxl/libxl_migration.c
index 678d850cf6..3e07f477c6 100644
--- a/src/libxl/libxl_migration.c
+++ b/src/libxl/libxl_migration.c
@@ -73,23 +73,23 @@ typedef struct _libxlMigrationDstArgs {
size_t nsocks;
} libxlMigrationDstArgs;
-G_DEFINE_TYPE(libxlMigrationDstArgs, libxl_migration_dst_args, G_TYPE_OBJECT);
#define LIBXL_TYPE_MIGRATION_DST_ARGS libxl_migration_dst_args_get_type()
G_DECLARE_FINAL_TYPE(libxlMigrationDstArgs,
libxl_migration_dst_args,
LIBXL,
MIGRATION_DST_ARGS,
GObject);
+G_DEFINE_TYPE(libxlMigrationDstArgs, libxl_migration_dst_args, G_TYPE_OBJECT);
static void libxlMigrationDstArgsFinalize(GObject *obj);
static void
-libxl_migration_dst_args_init(lixlMigrationDstArgs *args G_GNUC_UNUSED)
+libxl_migration_dst_args_init(libxlMigrationDstArgs *args G_GNUC_UNUSED)
{
}
static void
-libxl_migration_dst_args_class_init(lixlMigrationDstArgsClass *klass)
+libxl_migration_dst_args_class_init(libxlMigrationDstArgsClass *klass)
{
GObjectClass *obj = G_OBJECT_CLASS(klass);
@@ -608,9 +608,6 @@ libxlDomainMigrationDstPrepareTunnel3(virConnectPtr dconn,
goto endjob;
dataFD[1] = -1; /* 'st' owns the FD now & will close it */
-if (libxlMigrationDstArgsInitialize() < 0)
-goto endjob;
-
args = LIBXL_MIGRATION_DST_ARGS(
g_object_new(LIBXL_TYPE_MIGRATION_DST_ARGS, NULL));
@@ -772,9 +769,6 @@ libxlDomainMigrationDstPrepare(virConnectPtr dconn,
goto endjob;
}
-if (libxlMigrationDstArgsInitialize() < 0)
-goto endjob;
-
args = LIBXL_MIGRATION_DST_ARGS(
g_object_new(LIBXL_TYPE_MIGRATION_DST_ARGS, NULL));
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 v2 1/3] qemu: Track numa-mem-supported machine attribute
On a Thursday in 2020, Michal Privoznik wrote:
There is 'numa-mem-supported' machine attribute which specifies
whether '-numa mem=' is supported. Store it in our capabilities
as it will be used in later commits when building the command
line.
Signed-off-by: Michal Privoznik
---
src/qemu/qemu_capabilities.c | 44 ++-
src/qemu/qemu_capabilities.h | 3 +
src/qemu/qemu_capspriv.h | 3 +-
src/qemu/qemu_monitor.h | 1 +
src/qemu/qemu_monitor_json.c | 11 +
.../caps_1.5.3.x86_64.xml | 60 ++--
.../caps_1.6.0.x86_64.xml | 68 ++---
.../caps_1.7.0.x86_64.xml | 76 ++---
.../caps_2.1.1.x86_64.xml | 92 +++---
.../caps_2.10.0.aarch64.xml | 204 +++---
.../caps_2.10.0.ppc64.xml | 84 +++---
.../caps_2.10.0.s390x.xml | 28 +-
.../caps_2.10.0.x86_64.xml| 140 +-
.../caps_2.11.0.s390x.xml | 32 +--
.../caps_2.11.0.x86_64.xml| 140 +-
.../caps_2.12.0.aarch64.xml | 228 +++
.../caps_2.12.0.ppc64.xml | 100 +++
.../caps_2.12.0.s390x.xml | 36 +--
.../caps_2.12.0.x86_64.xml| 148 +-
.../caps_2.4.0.x86_64.xml | 116
.../caps_2.5.0.x86_64.xml | 124
.../caps_2.6.0.aarch64.xml| 164 +--
.../qemucapabilitiesdata/caps_2.6.0.ppc64.xml | 60 ++--
.../caps_2.6.0.x86_64.xml | 100 +++
.../qemucapabilitiesdata/caps_2.7.0.s390x.xml | 16 +-
.../caps_2.7.0.x86_64.xml | 108 +++
.../qemucapabilitiesdata/caps_2.8.0.s390x.xml | 20 +-
.../caps_2.8.0.x86_64.xml | 124
.../qemucapabilitiesdata/caps_2.9.0.ppc64.xml | 80 +++---
.../qemucapabilitiesdata/caps_2.9.0.s390x.xml | 24 +-
.../caps_2.9.0.x86_64.xml | 132 -
.../qemucapabilitiesdata/caps_3.0.0.ppc64.xml | 104 +++
.../caps_3.0.0.riscv32.xml| 10 +-
.../caps_3.0.0.riscv64.xml| 10 +-
.../qemucapabilitiesdata/caps_3.0.0.s390x.xml | 40 +--
.../caps_3.0.0.x86_64.xml | 156 +--
.../qemucapabilitiesdata/caps_3.1.0.ppc64.xml | 108 +++
.../caps_3.1.0.x86_64.xml | 164 +--
.../caps_4.0.0.aarch64.xml| 264 +-
.../qemucapabilitiesdata/caps_4.0.0.ppc64.xml | 112
.../caps_4.0.0.riscv32.xml| 10 +-
.../caps_4.0.0.riscv64.xml| 10 +-
.../qemucapabilitiesdata/caps_4.0.0.s390x.xml | 48 ++--
.../caps_4.0.0.x86_64.xml | 164 +--
.../caps_4.1.0.x86_64.xml | 176 ++--
.../caps_4.2.0.aarch64.xml| 52 ++--
.../qemucapabilitiesdata/caps_4.2.0.ppc64.xml | 72 ++---
.../caps_4.2.0.x86_64.xml | 184 ++--
.../caps_5.0.0.aarch64.xml| 52 ++--
.../qemucapabilitiesdata/caps_5.0.0.ppc64.xml | 76 ++---
.../caps_5.0.0.x86_64.xml | 176 ++--
.../caps_5.1.0.x86_64.xml | 176 ++--
tests/testutilsqemu.c | 6 +-
53 files changed, 2395 insertions(+), 2341 deletions(-)
diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c
index 7e711f22f8..2676fbab6f 100644
--- a/src/qemu/qemu_capabilities.c
+++ b/src/qemu/qemu_capabilities.c
@@ -594,6 +594,7 @@ struct _virQEMUCapsMachineType {
bool hotplugCpus;
bool qemuDefault;
char *defaultCPU;
+bool numaMemSupported;
};
typedef struct _virQEMUCapsHostCPUData virQEMUCapsHostCPUData;
@@ -1869,6 +1870,7 @@ virQEMUCapsAccelCopyMachineTypes(virQEMUCapsAccelPtr dst,
dst->machineTypes[i].maxCpus = src->machineTypes[i].maxCpus;
dst->machineTypes[i].hotplugCpus = src->machineTypes[i].hotplugCpus;
dst->machineTypes[i].qemuDefault = src->machineTypes[i].qemuDefault;
+dst->machineTypes[i].numaMemSupported =
src->machineTypes[i].numaMemSupported;
}
}
@@ -2510,6 +2512,28 @@ virQEMUCapsGetMachineDefaultCPU(virQEMUCapsPtr qemuCaps,
}
+bool
+virQEMUCapsGetMachineNumaMemSupported(virQEMUCapsPtr qemuCaps,
+ virDomainVirtType virtType,
+ const char *name)
+{
+virQEMUCapsAccelPtr accel;
+size_t i;
+
+if (!name)
+return 0;
+
The function returns bool, not int. Is it even possible to call it with
a null machine name?
Jano
+accel = virQEMUCapsGetAccel(qemuCaps, virtType);
+
+for (i = 0; i < accel->nmachineTypes; i++) {
+if (STREQ(accel->machineTypes[i].name, name))
+return accel->machineTypes[i].numaMemSupported;
+}
+
+return
Re: Firmware auto-select limitation
On 5/15/20 4:40 PM, GUOQING LI wrote: Hi everyone and Martin I would like to confirm the conversation we had in regard the possible limitation of firmware auto-select feature that’s been released since v5.20. I recall you saying that there were a lot of issues with auto select and later they shipped it into a Json file , it still didn’t solve all the problems, did it? I'm not aware of any pending fw autoselection bug/problem. Is it better to explicitly specify the loader and nvram path than using auto-select ? No. Just today, I encountered the issue of using firmware=“efi” on libvirt 5.4.0 If you specify the FW and NVRAM explicitly in the domain XML does this not reproduce? I am running Ubuntu eoan 19.10, I am wondering how did it happen. *Detailed error * Error starting domain: internal error: process exited while connecting to monitor: 2020-05-15T14:19:06.033267Z qemu-system-x86_64: -drive file=/usr/share/OVMF/OVMF_CODE.fd,if=pflash,format=raw,unit=0,readonly=on: Failed to lock byte 100 This error message comes from QEMU. Unfortunately, it doesn't say why locking the file failed. Is there perhaps some additional info in the audit log? I don't think this is related to FW autoselection (those bugs demonstrate in libvirt picking wrong FW image) and what you are facing is different. Michal
Re: [PATCH 33/40] libxl: convert libxlDriverConfig to GObject
On Wed, May 13, 2020 at 01:57:17PM +0200, Rafael Fonseca wrote: > Signed-off-by: Rafael Fonseca > --- > src/libxl/libxl_conf.c | 51 ++-- > src/libxl/libxl_conf.h | 12 ++- > src/libxl/libxl_driver.c| 162 > src/libxl/libxl_migration.c | 21 ++--- > tests/testutilsxen.c| 2 +- > 5 files changed, 96 insertions(+), 152 deletions(-) > > diff --git a/src/libxl/libxl_conf.c b/src/libxl/libxl_conf.c > index 2a719486fa..7684c4cb4b 100644 > --- a/src/libxl/libxl_conf.c > +++ b/src/libxl/libxl_conf.c This file needs diff --git a/src/libxl/libxl_conf.c b/src/libxl/libxl_conf.c index 7684c4cb4b..2cc3165a0a 100644 --- a/src/libxl/libxl_conf.c +++ b/src/libxl/libxl_conf.c @@ -1821,7 +1821,7 @@ libxlDriverConfigGet(libxlDriverPrivatePtr driver) libxlDriverConfigPtr cfg; libxlDriverLock(driver); -cfg = virObjectRef(driver->config); +cfg = g_object_ref(driver->config); libxlDriverUnlock(driver); return cfg; } 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 v2 2/3] qemuBuildNumaArgStr: Switch order of if() and for()
On a Thursday in 2020, Michal Privoznik wrote: When building -numa command line there is a for() loop that builds '-numa memdev=' for each guest NUMA node. And also records in a local variable whether any of memory-object-* backends must be used to satisfy desired config. Well, instead of checking in each iteration whether corresponding capabilities are set, we can do swap if() and for() and check only once. Fixes: https://bugzilla.redhat.com/show_bug.cgi?id=1783355 "Fixes" is confusing here, since this commit is a no-op. Signed-off-by: Michal Privoznik --- src/qemu/qemu_command.c | 8 1 file changed, 4 insertions(+), 4 deletions(-) Reviewed-by: Ján Tomko Jano signature.asc Description: PGP signature
Re: [PATCH v2 1/3] qemu: Track numa-mem-supported machine attribute
On a Thursday in 2020, Michal Privoznik wrote: There is 'numa-mem-supported' machine attribute which specifies whether '-numa mem=' is supported. Store it in our capabilities as it will be used in later commits when building the command line. Signed-off-by: Michal Privoznik --- src/qemu/qemu_capabilities.c | 44 ++- src/qemu/qemu_capabilities.h | 3 + src/qemu/qemu_capspriv.h | 3 +- src/qemu/qemu_monitor.h | 1 + src/qemu/qemu_monitor_json.c | 11 + .../caps_1.5.3.x86_64.xml | 60 ++-- .../caps_1.6.0.x86_64.xml | 68 ++--- .../caps_1.7.0.x86_64.xml | 76 ++--- .../caps_2.1.1.x86_64.xml | 92 +++--- .../caps_2.10.0.aarch64.xml | 204 +++--- .../caps_2.10.0.ppc64.xml | 84 +++--- .../caps_2.10.0.s390x.xml | 28 +- .../caps_2.10.0.x86_64.xml| 140 +- .../caps_2.11.0.s390x.xml | 32 +-- .../caps_2.11.0.x86_64.xml| 140 +- .../caps_2.12.0.aarch64.xml | 228 +++ .../caps_2.12.0.ppc64.xml | 100 +++ .../caps_2.12.0.s390x.xml | 36 +-- .../caps_2.12.0.x86_64.xml| 148 +- .../caps_2.4.0.x86_64.xml | 116 .../caps_2.5.0.x86_64.xml | 124 .../caps_2.6.0.aarch64.xml| 164 +-- .../qemucapabilitiesdata/caps_2.6.0.ppc64.xml | 60 ++-- .../caps_2.6.0.x86_64.xml | 100 +++ .../qemucapabilitiesdata/caps_2.7.0.s390x.xml | 16 +- .../caps_2.7.0.x86_64.xml | 108 +++ .../qemucapabilitiesdata/caps_2.8.0.s390x.xml | 20 +- .../caps_2.8.0.x86_64.xml | 124 .../qemucapabilitiesdata/caps_2.9.0.ppc64.xml | 80 +++--- .../qemucapabilitiesdata/caps_2.9.0.s390x.xml | 24 +- .../caps_2.9.0.x86_64.xml | 132 - .../qemucapabilitiesdata/caps_3.0.0.ppc64.xml | 104 +++ .../caps_3.0.0.riscv32.xml| 10 +- .../caps_3.0.0.riscv64.xml| 10 +- .../qemucapabilitiesdata/caps_3.0.0.s390x.xml | 40 +-- .../caps_3.0.0.x86_64.xml | 156 +-- .../qemucapabilitiesdata/caps_3.1.0.ppc64.xml | 108 +++ .../caps_3.1.0.x86_64.xml | 164 +-- .../caps_4.0.0.aarch64.xml| 264 +- .../qemucapabilitiesdata/caps_4.0.0.ppc64.xml | 112 .../caps_4.0.0.riscv32.xml| 10 +- .../caps_4.0.0.riscv64.xml| 10 +- .../qemucapabilitiesdata/caps_4.0.0.s390x.xml | 48 ++-- .../caps_4.0.0.x86_64.xml | 164 +-- .../caps_4.1.0.x86_64.xml | 176 ++-- .../caps_4.2.0.aarch64.xml| 52 ++-- .../qemucapabilitiesdata/caps_4.2.0.ppc64.xml | 72 ++--- .../caps_4.2.0.x86_64.xml | 184 ++-- .../caps_5.0.0.aarch64.xml| 52 ++-- .../qemucapabilitiesdata/caps_5.0.0.ppc64.xml | 76 ++--- .../caps_5.0.0.x86_64.xml | 176 ++-- .../caps_5.1.0.x86_64.xml | 176 ++-- tests/testutilsqemu.c | 6 +- 53 files changed, 2395 insertions(+), 2341 deletions(-) Reviewed-by: Ján Tomko Jano signature.asc Description: PGP signature
Re: [PATCH 00/40] convert virObjects to GObject
On Fri, May 15, 2020 at 02:34:01PM +0200, Michal Privoznik wrote: > On 5/13/20 1:56 PM, Rafael Fonseca wrote: > > This patch series convert various simple instances of virObject to a > > GObject equivalent. > > I'm sorry upfront for misusing your patchset and I'm also sorry for bringing > this up again. > > I think we need to step back and re-evaluate whether this is worth it. > GObject is horrible and frightening part of GLib. Not only one has to define > empty functions, but we can't mix virObject and GObject. For instance: > virObjectRef() called over GObject will corrupt memory. > > Worse, there is no way to check whether your patches converted everything > (and clearly they did not because I see couple of tests crashing; or maybe > they did at the time of send but now the code has changed - anyway, point > proven). > > I started reviewing and stopped at the first patch realizing, I have no idea > whether you converted every virObjectRef()/virObjectUnref() with > corresponding glib call. > > I also wanted to write a cocci spatch that might at least identify places > where that is happening, but apparently my spatch skills are poor. > > Does anybody have an idea how to verify these patches? My preferred option was to make virObject be a subclass of GObject. I tried this initially but hit a key problem - g_object_unref is void and virObjectUnref returns bool - true if any refs still exist. We rely on that for the virConnectPtr and virFDStream objects. I couldn't come up with a way to solve that at the time, but I've just had another think and believe we can solve it using a thread local set by the finalize. So I'll have another go at doing this inheritance. 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 v2 11/13] audio: deprecate -soundhw pcspk
On a Friday in 2020, Gerd Hoffmann wrote:
Add deprecation message to the audio init function.
Factor out audio initialization and call that from
both audio init and realize, so setting audiodev via
-global is enough to properly initialize pcspk.
Signed-off-by: Gerd Hoffmann
---
hw/audio/pcspk.c | 24 +---
1 file changed, 21 insertions(+), 3 deletions(-)
@@ -236,9 +245,18 @@ static const TypeInfo pcspk_info = {
.class_init = pcspk_class_initfn,
};
+static int pcspk_audio_init_soundhw(ISABus *bus)
+{
+PCSpkState *s = pcspk_state;
+
+warn_report("'-soundhw pcspk' is deprecated, "
+"please set a backend using '-global isa-pcspk.audiodev='
instead");
+return pcspk_audio_init(s);
-soundhw pcspk is the only soundhw device present in libvirt git.
Is there a way to probe for this change via QMP?
Jano
+}
+
static void pcspk_register(void)
{
type_register_static(_info);
-isa_register_soundhw("pcspk", "PC speaker", pcspk_audio_init);
+isa_register_soundhw("pcspk", "PC speaker", pcspk_audio_init_soundhw);
}
type_init(pcspk_register)
--
2.18.4
signature.asc
Description: PGP signature
Re: [PATCH 29/40] datatypes: convert virDomain to GObject
On Wed, May 13, 2020 at 01:57:13PM +0200, Rafael Fonseca wrote:
> Signed-off-by: Rafael Fonseca
> ---
> src/conf/domain_event.c | 58 ++--
> src/datatypes.c | 50 ++
> src/datatypes.h | 20 ++--
> src/esx/esx_driver.c| 7 +-
> src/hyperv/hyperv_driver.c | 8 +-
> src/libvirt-domain.c| 6 +-
> src/libvirt_private.syms| 2 +-
> src/libxl/libxl_migration.c | 3 +-
> src/locking/sanlock_helper.c| 5 +-
> src/qemu/qemu_driver.c | 6 +-
> src/qemu/qemu_migration.c | 6 +-
> src/remote/remote_daemon_dispatch.c | 139 +---
> src/remote/remote_driver.c | 100 ++--
> src/rpc/gendispatch.pl | 6 +-
> src/vbox/vbox_common.c | 11 +--
> src/vz/vz_driver.c | 5 +-
> 16 files changed, 161 insertions(+), 271 deletions(-)
>
> diff --git a/src/datatypes.c b/src/datatypes.c
> index 1c8eff9685..0af5c326a1 100644
> --- a/src/datatypes.c
> +++ b/src/datatypes.c
> @@ -1133,7 +1143,7 @@ virGetDomainCheckpoint(virDomainPtr domain,
> ret = VIR_DOMAIN_CHECKPOINT(g_object_new(VIR_TYPE_DOMAIN_CHECKPOINT,
> NULL));
> ret->name = g_strdup(name);
>
> -ret->domain = virObjectRef(domain);
> +ret->domain = g_object_ref(domain);
>
> return g_steal_pointer();
> }
> @@ -1186,7 +1196,7 @@ virGetDomainSnapshot(virDomainPtr domain, const char
> *name)
> ret = VIR_DOMAIN_SNAPSHOT(g_object_new(VIR_TYPE_DOMAIN_SNAPSHOT, NULL));
> ret->name = g_strdup(name);
>
> -ret->domain = virObjectRef(domain);
> +ret->domain = g_object_ref(domain);
>
> return g_steal_pointer();
> }
Missed the unref side
diff --git a/src/datatypes.c b/src/datatypes.c
index 0af5c326a1..cf3a8e7857 100644
--- a/src/datatypes.c
+++ b/src/datatypes.c
@@ -1166,7 +1166,7 @@ virDomainCheckpointFinalize(GObject *obj)
VIR_DEBUG("release checkpoint %p %s", checkpoint, checkpoint->name);
VIR_FREE(checkpoint->name);
-virObjectUnref(checkpoint->domain);
+g_clear_object(>domain);
G_OBJECT_CLASS(vir_domain_checkpoint_parent_class)->finalize(obj);
}
@@ -1219,7 +1219,7 @@ virDomainSnapshotFinalize(GObject *obj)
VIR_DEBUG("release snapshot %p %s", snapshot, snapshot->name);
VIR_FREE(snapshot->name);
-virObjectUnref(snapshot->domain);
+g_clear_object(>domain);
G_OBJECT_CLASS(vir_domain_snapshot_parent_class)->finalize(obj);
}
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 :|
Firmware auto-select limitation
Hi everyone and Martin
I would like to confirm the conversation we had in regard the possible
limitation of firmware auto-select feature that’s been released since v5.20. I
recall you saying that there were a lot of issues with auto select and later
they shipped it into a Json file , it still didn’t solve all the problems, did
it?
Is it better to explicitly specify the loader and nvram path than using
auto-select ?
Just today, I encountered the issue of using firmware=“efi” on libvirt 5.4.0
I am running Ubuntu eoan 19.10, I am wondering how did it happen.
Detailed error
Error starting domain: internal error: process exited while connecting to
monitor: 2020-05-15T14:19:06.033267Z qemu-system-x86_64: -drive
file=/usr/share/OVMF/OVMF_CODE.fd,if=pflash,format=raw,unit=0,readonly=on:
Failed to lock byte 100
Traceback (most recent call last):
File "/usr/share/virt-manager/virtManager/asyncjob.py", line 75, in cb_wrapper
callback(asyncjob, *args, **kwargs)
File "/usr/share/virt-manager/virtManager/asyncjob.py", line 111, in tmpcb
callback(*args, **kwargs)
File "/usr/share/virt-manager/virtManager/object/libvirtobject.py", line 66,
in newfn
ret = fn(self, *args, **kwargs)
File "/usr/share/virt-manager/virtManager/object/domain.py", line 1279, in
startup
self._backend.create()
File "/usr/lib/python3/dist-packages/libvirt.py", line 1080, in create
if ret == -1: raise libvirtError ('virDomainCreate() failed', dom=self)
libvirt.libvirtError: internal error: process exited while connecting to
monitor: 2020-05-15T14:19:06.033267Z qemu-system-x86_64: -drive
file=/usr/share/OVMF/OVMF_CODE.fd,if=pflash,format=raw,unit=0,readonly=on:
Failed to lock byte 100
[PATCH v2 08/13] audio: deprecate -soundhw gus
Switch to deprecated_register_soundhw().
Remove the now obsolete init function.
Signed-off-by: Gerd Hoffmann
---
hw/audio/gus.c | 8 +---
1 file changed, 1 insertion(+), 7 deletions(-)
diff --git a/hw/audio/gus.c b/hw/audio/gus.c
index eb4a803fb53b..61d16fad9ffb 100644
--- a/hw/audio/gus.c
+++ b/hw/audio/gus.c
@@ -292,12 +292,6 @@ static void gus_realizefn (DeviceState *dev, Error **errp)
AUD_set_active_out (s->voice, 1);
}
-static int GUS_init (ISABus *bus)
-{
-isa_create_simple (bus, TYPE_GUS);
-return 0;
-}
-
static Property gus_properties[] = {
DEFINE_AUDIO_PROPERTIES(GUSState, card),
DEFINE_PROP_UINT32 ("freq",GUSState, freq,44100),
@@ -328,7 +322,7 @@ static const TypeInfo gus_info = {
static void gus_register_types (void)
{
type_register_static (_info);
-isa_register_soundhw("gus", "Gravis Ultrasound GF1", GUS_init);
+deprecated_register_soundhw("gus", "Gravis Ultrasound GF1", 1, TYPE_GUS);
}
type_init (gus_register_types)
--
2.18.4
[PATCH v2 07/13] audio: deprecate -soundhw cs4231a
Switch to deprecated_register_soundhw().
Remove the now obsolete init function.
Signed-off-by: Gerd Hoffmann
---
hw/audio/cs4231a.c | 8 +---
1 file changed, 1 insertion(+), 7 deletions(-)
diff --git a/hw/audio/cs4231a.c b/hw/audio/cs4231a.c
index ffdbb58d6a11..59705a8d4701 100644
--- a/hw/audio/cs4231a.c
+++ b/hw/audio/cs4231a.c
@@ -683,12 +683,6 @@ static void cs4231a_realizefn (DeviceState *dev, Error
**errp)
AUD_register_card ("cs4231a", >card);
}
-static int cs4231a_init (ISABus *bus)
-{
-isa_create_simple (bus, TYPE_CS4231A);
-return 0;
-}
-
static Property cs4231a_properties[] = {
DEFINE_AUDIO_PROPERTIES(CSState, card),
DEFINE_PROP_UINT32 ("iobase", CSState, port, 0x534),
@@ -720,7 +714,7 @@ static const TypeInfo cs4231a_info = {
static void cs4231a_register_types (void)
{
type_register_static (_info);
-isa_register_soundhw("cs4231a", "CS4231A", cs4231a_init);
+deprecated_register_soundhw("cs4231a", "CS4231A", 1, TYPE_CS4231A);
}
type_init (cs4231a_register_types)
--
2.18.4
[PATCH v2 10/13] audio: deprecate -soundhw hda
Add deprecation message to the audio init function.
Signed-off-by: Gerd Hoffmann
---
hw/audio/intel-hda.c | 3 +++
1 file changed, 3 insertions(+)
diff --git a/hw/audio/intel-hda.c b/hw/audio/intel-hda.c
index 4696ae0d9a61..d491e407b317 100644
--- a/hw/audio/intel-hda.c
+++ b/hw/audio/intel-hda.c
@@ -25,6 +25,7 @@
#include "qemu/bitops.h"
#include "qemu/log.h"
#include "qemu/module.h"
+#include "qemu/error-report.h"
#include "hw/audio/soundhw.h"
#include "intel-hda.h"
#include "migration/vmstate.h"
@@ -1307,6 +1308,8 @@ static int intel_hda_and_codec_init(PCIBus *bus)
BusState *hdabus;
DeviceState *codec;
+warn_report("'-soundhw hda' is deprecated, "
+"please use '-device intel-hda -device hda-duplex' instead");
controller = DEVICE(pci_create_simple(bus, -1, "intel-hda"));
hdabus = QLIST_FIRST(>child_bus);
codec = qdev_create(hdabus, "hda-duplex");
--
2.18.4
[PATCH v2 05/13] audio: deprecate -soundhw es1370
Switch to deprecated_register_soundhw(). Remove the now obsolete init
function. Add an alias so both es1370 and ES1370 are working with
-device.
Signed-off-by: Gerd Hoffmann
---
hw/audio/es1370.c | 9 ++---
qdev-monitor.c| 1 +
2 files changed, 3 insertions(+), 7 deletions(-)
diff --git a/hw/audio/es1370.c b/hw/audio/es1370.c
index 89c4dabcd44f..f36ed95ac93f 100644
--- a/hw/audio/es1370.c
+++ b/hw/audio/es1370.c
@@ -881,12 +881,6 @@ static void es1370_exit(PCIDevice *dev)
AUD_remove_card(>card);
}
-static int es1370_init (PCIBus *bus)
-{
-pci_create_simple (bus, -1, TYPE_ES1370);
-return 0;
-}
-
static Property es1370_properties[] = {
DEFINE_AUDIO_PROPERTIES(ES1370State, card),
DEFINE_PROP_END_OF_LIST(),
@@ -925,7 +919,8 @@ static const TypeInfo es1370_info = {
static void es1370_register_types (void)
{
type_register_static (_info);
-pci_register_soundhw("es1370", "ENSONIQ AudioPCI ES1370", es1370_init);
+deprecated_register_soundhw("es1370", "ENSONIQ AudioPCI ES1370",
+0, TYPE_ES1370);
}
type_init (es1370_register_types)
diff --git a/qdev-monitor.c b/qdev-monitor.c
index 9c3cb89473c6..748733215004 100644
--- a/qdev-monitor.c
+++ b/qdev-monitor.c
@@ -55,6 +55,7 @@ typedef struct QDevAlias
static const QDevAlias qdev_alias_table[] = {
{ "AC97", "ac97" }, /* -soundhw name */
{ "e1000", "e1000-82540em" },
+{ "ES1370", "es1370" }, /* -soundhw name */
{ "ich9-ahci", "ahci" },
{ "lsi53c895a", "lsi" },
{ "virtio-9p-ccw", "virtio-9p", QEMU_ARCH_S390X },
--
2.18.4
[PATCH v2 12/13] audio: add soundhw deprecation notice
Signed-off-by: Gerd Hoffmann --- docs/system/deprecated.rst | 9 + 1 file changed, 9 insertions(+) diff --git a/docs/system/deprecated.rst b/docs/system/deprecated.rst index 3142fac38658..7de1045b7e27 100644 --- a/docs/system/deprecated.rst +++ b/docs/system/deprecated.rst @@ -88,6 +88,15 @@ should specify an ``audiodev=`` property. Additionally, when using vnc, you should specify an ``audiodev=`` propery if you plan to transmit audio through the VNC protocol. +Creating sound card devices using ``-soundhw`` (since 5.1) +'' + +Sound card devices should be created using ``-device`` instead. The +names are the same for most devices. The exceptions are ``hda`` which +needs two devices (``-device intel-hda --device hda-duplex``) and +``pcspk`` which can be activated using ``-global +pcspk.audiodev=``. + ``-mon ...,control=readline,pretty=on|off`` (since 4.1) ''' -- 2.18.4
[PATCH v2 06/13] audio: deprecate -soundhw adlib
Switch to deprecated_register_soundhw().
Remove the now obsolete init function.
Signed-off-by: Gerd Hoffmann
---
hw/audio/adlib.c | 8 +---
1 file changed, 1 insertion(+), 7 deletions(-)
diff --git a/hw/audio/adlib.c b/hw/audio/adlib.c
index 7c3b67dcfb8c..65dff5b6fca4 100644
--- a/hw/audio/adlib.c
+++ b/hw/audio/adlib.c
@@ -319,16 +319,10 @@ static const TypeInfo adlib_info = {
.class_init= adlib_class_initfn,
};
-static int Adlib_init (ISABus *bus)
-{
-isa_create_simple (bus, TYPE_ADLIB);
-return 0;
-}
-
static void adlib_register_types (void)
{
type_register_static (_info);
-isa_register_soundhw("adlib", ADLIB_DESC, Adlib_init);
+deprecated_register_soundhw("adlib", ADLIB_DESC, 1, TYPE_ADLIB);
}
type_init (adlib_register_types)
--
2.18.4
[PATCH v2 01/13] stubs: add isa_create_simple
Needed for -soundhw cleanup.
Signed-off-by: Gerd Hoffmann
Reviewed-by: Philippe Mathieu-Daudé
Tested-by: Philippe Mathieu-Daudé
---
stubs/isa-bus.c | 7 +++
stubs/Makefile.objs | 1 +
2 files changed, 8 insertions(+)
create mode 100644 stubs/isa-bus.c
diff --git a/stubs/isa-bus.c b/stubs/isa-bus.c
new file mode 100644
index ..522f448997d4
--- /dev/null
+++ b/stubs/isa-bus.c
@@ -0,0 +1,7 @@
+#include "qemu/osdep.h"
+#include "hw/isa/isa.h"
+
+ISADevice *isa_create_simple(ISABus *bus, const char *name)
+{
+g_assert_not_reached();
+}
diff --git a/stubs/Makefile.objs b/stubs/Makefile.objs
index 45be5dc0ed78..c61ff38cc801 100644
--- a/stubs/Makefile.objs
+++ b/stubs/Makefile.objs
@@ -13,6 +13,7 @@ stub-obj-y += get-vm-name.o
stub-obj-y += iothread.o
stub-obj-y += iothread-lock.o
stub-obj-y += is-daemonized.o
+stub-obj-y += isa-bus.o
stub-obj-$(CONFIG_LINUX_AIO) += linux-aio.o
stub-obj-$(CONFIG_LINUX_IO_URING) += io_uring.o
stub-obj-y += machine-init-done.o
--
2.18.4
[PATCH v2 00/13] audio: deprecate -soundhw
v2: - use g_assert_not_reached() for stubs. - add deprecation notice. Gerd Hoffmann (13): stubs: add isa_create_simple stubs: add pci_create_simple audio: add deprecated_register_soundhw audio: deprecate -soundhw ac97 audio: deprecate -soundhw es1370 audio: deprecate -soundhw adlib audio: deprecate -soundhw cs4231a audio: deprecate -soundhw gus audio: deprecate -soundhw sb16 audio: deprecate -soundhw hda audio: deprecate -soundhw pcspk audio: add soundhw deprecation notice [RFC] audio: try use onboard audiodev for pcspk include/hw/audio/soundhw.h | 2 ++ hw/audio/ac97.c| 9 ++--- hw/audio/adlib.c | 8 +--- hw/audio/cs4231a.c | 8 +--- hw/audio/es1370.c | 9 ++--- hw/audio/gus.c | 8 +--- hw/audio/intel-hda.c | 3 +++ hw/audio/pcspk.c | 27 --- hw/audio/sb16.c| 9 ++--- hw/audio/soundhw.c | 24 +++- qdev-monitor.c | 2 ++ stubs/isa-bus.c| 7 +++ stubs/pci-bus.c| 7 +++ docs/system/deprecated.rst | 9 + stubs/Makefile.objs| 2 ++ 15 files changed, 88 insertions(+), 46 deletions(-) create mode 100644 stubs/isa-bus.c create mode 100644 stubs/pci-bus.c -- 2.18.4
[PATCH v2 11/13] audio: deprecate -soundhw pcspk
Add deprecation message to the audio init function.
Factor out audio initialization and call that from
both audio init and realize, so setting audiodev via
-global is enough to properly initialize pcspk.
Signed-off-by: Gerd Hoffmann
---
hw/audio/pcspk.c | 24 +---
1 file changed, 21 insertions(+), 3 deletions(-)
diff --git a/hw/audio/pcspk.c b/hw/audio/pcspk.c
index c37a3878612e..ab290e686783 100644
--- a/hw/audio/pcspk.c
+++ b/hw/audio/pcspk.c
@@ -28,6 +28,7 @@
#include "audio/audio.h"
#include "qemu/module.h"
#include "qemu/timer.h"
+#include "qemu/error-report.h"
#include "hw/timer/i8254.h"
#include "migration/vmstate.h"
#include "hw/audio/pcspk.h"
@@ -112,11 +113,15 @@ static void pcspk_callback(void *opaque, int free)
}
}
-static int pcspk_audio_init(ISABus *bus)
+static int pcspk_audio_init(PCSpkState *s)
{
-PCSpkState *s = pcspk_state;
struct audsettings as = {PCSPK_SAMPLE_RATE, 1, AUDIO_FORMAT_U8, 0};
+if (s->voice) {
+/* already initialized */
+return 0;
+}
+
AUD_register_card(s_spk, >card);
s->voice = AUD_open_out(>card, s->voice, s_spk, s, pcspk_callback, );
@@ -185,6 +190,10 @@ static void pcspk_realizefn(DeviceState *dev, Error **errp)
isa_register_ioport(isadev, >ioport, s->iobase);
+if (s->card.state) {
+pcspk_audio_init(s);
+}
+
pcspk_state = s;
}
@@ -236,9 +245,18 @@ static const TypeInfo pcspk_info = {
.class_init = pcspk_class_initfn,
};
+static int pcspk_audio_init_soundhw(ISABus *bus)
+{
+PCSpkState *s = pcspk_state;
+
+warn_report("'-soundhw pcspk' is deprecated, "
+"please set a backend using '-global
isa-pcspk.audiodev=' instead");
+return pcspk_audio_init(s);
+}
+
static void pcspk_register(void)
{
type_register_static(_info);
-isa_register_soundhw("pcspk", "PC speaker", pcspk_audio_init);
+isa_register_soundhw("pcspk", "PC speaker", pcspk_audio_init_soundhw);
}
type_init(pcspk_register)
--
2.18.4
[PATCH v2 04/13] audio: deprecate -soundhw ac97
Switch to deprecated_register_soundhw(). Remove the now obsolete init
function. Add an alias so both ac97 and AC97 are working with -device.
Signed-off-by: Gerd Hoffmann
---
hw/audio/ac97.c | 9 ++---
qdev-monitor.c | 1 +
2 files changed, 3 insertions(+), 7 deletions(-)
diff --git a/hw/audio/ac97.c b/hw/audio/ac97.c
index 8a9b9924c495..38522cf0ba44 100644
--- a/hw/audio/ac97.c
+++ b/hw/audio/ac97.c
@@ -1393,12 +1393,6 @@ static void ac97_exit(PCIDevice *dev)
AUD_remove_card(>card);
}
-static int ac97_init (PCIBus *bus)
-{
-pci_create_simple(bus, -1, TYPE_AC97);
-return 0;
-}
-
static Property ac97_properties[] = {
DEFINE_AUDIO_PROPERTIES(AC97LinkState, card),
DEFINE_PROP_END_OF_LIST (),
@@ -1436,7 +1430,8 @@ static const TypeInfo ac97_info = {
static void ac97_register_types (void)
{
type_register_static (_info);
-pci_register_soundhw("ac97", "Intel 82801AA AC97 Audio", ac97_init);
+deprecated_register_soundhw("ac97", "Intel 82801AA AC97 Audio",
+0, TYPE_AC97);
}
type_init (ac97_register_types)
diff --git a/qdev-monitor.c b/qdev-monitor.c
index a4735d3bb190..9c3cb89473c6 100644
--- a/qdev-monitor.c
+++ b/qdev-monitor.c
@@ -53,6 +53,7 @@ typedef struct QDevAlias
/* Please keep this table sorted by typename. */
static const QDevAlias qdev_alias_table[] = {
+{ "AC97", "ac97" }, /* -soundhw name */
{ "e1000", "e1000-82540em" },
{ "ich9-ahci", "ahci" },
{ "lsi53c895a", "lsi" },
--
2.18.4
[PATCH v2 02/13] stubs: add pci_create_simple
Needed for -soundhw cleanup.
Signed-off-by: Gerd Hoffmann
Reviewed-by: Philippe Mathieu-Daudé
Tested-by: Philippe Mathieu-Daudé
---
stubs/pci-bus.c | 7 +++
stubs/Makefile.objs | 1 +
2 files changed, 8 insertions(+)
create mode 100644 stubs/pci-bus.c
diff --git a/stubs/pci-bus.c b/stubs/pci-bus.c
new file mode 100644
index ..a8932fa93250
--- /dev/null
+++ b/stubs/pci-bus.c
@@ -0,0 +1,7 @@
+#include "qemu/osdep.h"
+#include "hw/pci/pci.h"
+
+PCIDevice *pci_create_simple(PCIBus *bus, int devfn, const char *name)
+{
+g_assert_not_reached();
+}
diff --git a/stubs/Makefile.objs b/stubs/Makefile.objs
index c61ff38cc801..5e7f2b3f061e 100644
--- a/stubs/Makefile.objs
+++ b/stubs/Makefile.objs
@@ -40,6 +40,7 @@ stub-obj-y += target-get-monitor-def.o
stub-obj-y += vmgenid.o
stub-obj-y += xen-common.o
stub-obj-y += xen-hvm.o
+stub-obj-y += pci-bus.o
stub-obj-y += pci-host-piix.o
stub-obj-y += ram-block.o
stub-obj-y += ramfb.o
--
2.18.4
[PATCH v2 09/13] audio: deprecate -soundhw sb16
Switch to deprecated_register_soundhw().
Remove the now obsolete init function.
Signed-off-by: Gerd Hoffmann
---
hw/audio/sb16.c | 9 ++---
1 file changed, 2 insertions(+), 7 deletions(-)
diff --git a/hw/audio/sb16.c b/hw/audio/sb16.c
index df6f755a37f8..2d9e50f99b5d 100644
--- a/hw/audio/sb16.c
+++ b/hw/audio/sb16.c
@@ -1415,12 +1415,6 @@ static void sb16_realizefn (DeviceState *dev, Error
**errp)
AUD_register_card ("sb16", >card);
}
-static int SB16_init (ISABus *bus)
-{
-isa_create_simple (bus, TYPE_SB16);
-return 0;
-}
-
static Property sb16_properties[] = {
DEFINE_AUDIO_PROPERTIES(SB16State, card),
DEFINE_PROP_UINT32 ("version", SB16State, ver, 0x0405), /* 4.5 */
@@ -1453,7 +1447,8 @@ static const TypeInfo sb16_info = {
static void sb16_register_types (void)
{
type_register_static (_info);
-isa_register_soundhw("sb16", "Creative Sound Blaster 16", SB16_init);
+deprecated_register_soundhw("sb16", "Creative Sound Blaster 16",
+1, TYPE_SB16);
}
type_init (sb16_register_types)
--
2.18.4
[PATCH v2 13/13] [RFC] audio: try use onboard audiodev for pcspk
New naming convention: Use "onboard" audiodev for onboard audio
devices, using "-audiodev pa,id=onboard" for example.
This patchs implements it for pcspk, it will try to use "onboard" by
default. Setting another name using "-global pcspk.audiodev="
continues to work.
If we want go this route we should do the same for other onboard
audio devices too (arm boards, ...).
Signed-off-by: Gerd Hoffmann
---
hw/audio/pcspk.c | 3 +++
docs/system/deprecated.rst | 4 ++--
2 files changed, 5 insertions(+), 2 deletions(-)
diff --git a/hw/audio/pcspk.c b/hw/audio/pcspk.c
index ab290e686783..9a08e9d8e05b 100644
--- a/hw/audio/pcspk.c
+++ b/hw/audio/pcspk.c
@@ -190,6 +190,9 @@ static void pcspk_realizefn(DeviceState *dev, Error **errp)
isa_register_ioport(isadev, >ioport, s->iobase);
+if (!s->card.state) {
+s->card.state = audio_state_by_name("onboard");
+}
if (s->card.state) {
pcspk_audio_init(s);
}
diff --git a/docs/system/deprecated.rst b/docs/system/deprecated.rst
index 7de1045b7e27..34312fc0a963 100644
--- a/docs/system/deprecated.rst
+++ b/docs/system/deprecated.rst
@@ -94,8 +94,8 @@ Creating sound card devices using ``-soundhw`` (since 5.1)
Sound card devices should be created using ``-device`` instead. The
names are the same for most devices. The exceptions are ``hda`` which
needs two devices (``-device intel-hda --device hda-duplex``) and
-``pcspk`` which can be activated using ``-global
-pcspk.audiodev=``.
+``pcspk`` which can be activated by creating an audiodev named
+``onboard``.
``-mon ...,control=readline,pretty=on|off`` (since 4.1)
'''
--
2.18.4
[PATCH v2 03/13] audio: add deprecated_register_soundhw
Add helper function for -soundhw deprecation. It can replace the
simple init functions which just call {isa,pci}_create_simple()
with a hardcoded type. It also prints a deprecation message.
Signed-off-by: Gerd Hoffmann
---
include/hw/audio/soundhw.h | 2 ++
hw/audio/soundhw.c | 24 +++-
2 files changed, 25 insertions(+), 1 deletion(-)
diff --git a/include/hw/audio/soundhw.h b/include/hw/audio/soundhw.h
index c8eef8241846..f09a297854af 100644
--- a/include/hw/audio/soundhw.h
+++ b/include/hw/audio/soundhw.h
@@ -6,6 +6,8 @@ void isa_register_soundhw(const char *name, const char *descr,
void pci_register_soundhw(const char *name, const char *descr,
int (*init_pci)(PCIBus *bus));
+void deprecated_register_soundhw(const char *name, const char *descr,
+ int isa, const char *typename);
void soundhw_init(void);
void select_soundhw(const char *optarg);
diff --git a/hw/audio/soundhw.c b/hw/audio/soundhw.c
index c750473c8f0c..173b674ff53a 100644
--- a/hw/audio/soundhw.c
+++ b/hw/audio/soundhw.c
@@ -22,6 +22,7 @@
* THE SOFTWARE.
*/
#include "qemu/osdep.h"
+#include "qemu/option.h"
#include "qemu/help_option.h"
#include "qemu/error-report.h"
#include "qom/object.h"
@@ -32,6 +33,7 @@
struct soundhw {
const char *name;
const char *descr;
+const char *typename;
int enabled;
int isa;
union {
@@ -65,6 +67,17 @@ void pci_register_soundhw(const char *name, const char
*descr,
soundhw_count++;
}
+void deprecated_register_soundhw(const char *name, const char *descr,
+ int isa, const char *typename)
+{
+assert(soundhw_count < ARRAY_SIZE(soundhw) - 1);
+soundhw[soundhw_count].name = name;
+soundhw[soundhw_count].descr = descr;
+soundhw[soundhw_count].isa = isa;
+soundhw[soundhw_count].typename = typename;
+soundhw_count++;
+}
+
void select_soundhw(const char *optarg)
{
struct soundhw *c;
@@ -136,7 +149,16 @@ void soundhw_init(void)
for (c = soundhw; c->name; ++c) {
if (c->enabled) {
-if (c->isa) {
+if (c->typename) {
+warn_report("'-soundhw %s' is deprecated, "
+"please use '-device %s' instead",
+c->name, c->typename);
+if (c->isa) {
+isa_create_simple(isa_bus, c->typename);
+} else {
+pci_create_simple(pci_bus, -1, c->typename);
+}
+} else if (c->isa) {
if (!isa_bus) {
error_report("ISA bus not available for %s", c->name);
exit(1);
--
2.18.4
Re: [PATCH 1/6] util: introduce a parser for kernel cmdline arguments
On Mon, May 11, 2020 at 06:41:56PM +0200, Boris Fiuczynski wrote:
> From: Paulo de Rezende Pinatti
>
> Introduce two utility functions to parse a kernel command
> line string according to the kernel code parsing rules in
> order to enable the caller to perform operations such as
> verifying whether certain argument=value combinations are
> present or retrieving an argument's value.
>
> Signed-off-by: Paulo de Rezende Pinatti
> Reviewed-by: Boris Fiuczynski
> ---
> src/libvirt_private.syms | 2 +
> src/util/virutil.c | 173 +++
> src/util/virutil.h | 18
> tests/utiltest.c | 130 +
> 4 files changed, 323 insertions(+)
>
> diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
> index 935ef7303b..25b22a3e3f 100644
> --- a/src/libvirt_private.syms
> +++ b/src/libvirt_private.syms
> @@ -3428,6 +3428,8 @@ virHostGetDRMRenderNode;
> virHostHasIOMMU;
> virIndexToDiskName;
> virIsDevMapperDevice;
> +virKernelCmdlineGetValue;
> +virKernelCmdlineMatchParam;
> virMemoryLimitIsSet;
> virMemoryLimitTruncate;
> virMemoryMaxValue;
> diff --git a/src/util/virutil.c b/src/util/virutil.c
> index fb46501142..264aae8d01 100644
> --- a/src/util/virutil.c
> +++ b/src/util/virutil.c
> @@ -1725,6 +1725,179 @@ virHostGetDRMRenderNode(void)
> return ret;
> }
>
> +
> +#define VIR_IS_CMDLINE_SPACE(value) \
> +(g_ascii_isspace(value) || (unsigned char) value == 160)
Hmm, we're not checking the non-breaking space anywhere else in the code, so I
think we'd be fine not checking it here either, so plain g_ascii_isspace()
would suffice in the code
> +
> +
> +static bool virKernelArgsAreEqual(const char *arg1,
> + const char *arg2,
> + size_t n)
> +{
> +size_t i;
> +
> +for (i = 0; i < n; i++) {
> +if ((arg1[i] == '-' && arg2[i] == '_') ||
> +(arg1[i] == '_' && arg2[i] == '-') || (arg1[i] == arg2[i])) {
Because '-' and '_' are equal in the parameter syntax, rather than introducing
another string equality function just because of this, we should normalize the
inputs by replacing occurrences of one with the other and then simply do STREQ
at the appropriate spot in the code
> +if (arg1[i] == '\0')
> +return true;
> +continue;
> +}
> +return false;
> +}
> +return true;
> +}
> +
> +
> +/*
> + * Parse the kernel cmdline and store the value of @arg in @val
> + * which can be NULL if @arg has no value or if it's not found.
> + * In addition, store in @next the address right after
> + * @arg=@value for possible further processing.
> + *
> + * @arg: kernel command line argument
> + * @cmdline: kernel command line string to be checked for @arg
> + * @val: pointer to hold retrieved value of @arg
> + * @next: pointer to hold address right after @arg=@val, will
> + * point to the string's end (NULL) in case @arg is not found
> + *
> + * Returns 0 if @arg is present in @cmdline, 1 otherwise
> + */
> +int virKernelCmdlineGetValue(const char *arg,
> + const char *cmdline,
> + char **val,
> + const char **next)
> +{
> +size_t i = 0, param_i;
in this specific case I think that naming the iteration variable param_i is
more confusing than actually settling down with something like "offset"
especially when you're doing arithmetics with it.
> +size_t arg_len = strlen(arg);
> +bool is_quoted, match;
1 declaration/definition per line please
> +
> +*next = cmdline;
> +*val = NULL;
> +
> +while (cmdline[i] != '\0') {
> +/* remove leading spaces */
> +while (VIR_IS_CMDLINE_SPACE(cmdline[i]))
> +i++;
For ^this, we already have a primitive virSkipSpaces()
> +if (cmdline[i] == '"') {
> +i++;
> +is_quoted = true;
> +} else {
> +is_quoted = false;
> +}
> +for (param_i = i; cmdline[param_i]; param_i++) {
> +if ((VIR_IS_CMDLINE_SPACE(cmdline[param_i]) && !is_quoted) ||
> +cmdline[param_i] == '=')
> +break;
> +if (cmdline[param_i] == '"')
> +is_quoted = !is_quoted;
> +}
> +if (param_i-i == arg_len && virKernelArgsAreEqual(cmdline+i, arg,
> arg_len))
We don't use Yoda conditions, so ^this should be arg_len == param_i - i
> +match = true;
> +else
> +match = false;
> +/* arg has no value */
> +if (cmdline[param_i] != '=') {
> +if (match) {
> +*next = cmdline+param_i;
please use a space in between the operands and the operator
> +return 0;
> +}
> +i = param_i;
> +continue;
> +}
> +param_i++;
> +if (cmdline[param_i] == '\0')
> +break;
>
[libvirt PATCH] docs: Update after libvirt-ruby repository rename
Signed-off-by: Andrea Bolognani --- Pushed as trivial. docs/ci.rst| 10 +- docs/downloads.html.in | 8 2 files changed, 9 insertions(+), 9 deletions(-) diff --git a/docs/ci.rst b/docs/ci.rst index fe59b98923..2a8788d0d7 100644 --- a/docs/ci.rst +++ b/docs/ci.rst @@ -76,16 +76,16 @@ Language bindings :target: https://gitlab.com/libvirt/libvirt-python/pipelines :alt: libvirt-python pipeline status + * - libvirt-ruby + - .. image:: https://gitlab.com/libvirt/libvirt-ruby/badges/master/pipeline.svg + :target: https://gitlab.com/libvirt/libvirt-ruby/pipelines + :alt: libvirt-ruby pipeline status + * - libvirt-rust - .. image:: https://gitlab.com/libvirt/libvirt-rust/badges/master/pipeline.svg :target: https://gitlab.com/libvirt/libvirt-rust/pipelines :alt: libvirt-rust pipeline status - * - ruby-libvirt - - .. image:: https://gitlab.com/libvirt/ruby-libvirt/badges/master/pipeline.svg - :target: https://gitlab.com/libvirt/ruby-libvirt/pipelines - :alt: ruby-libvirt pipeline status - Object mappings --- diff --git a/docs/downloads.html.in b/docs/downloads.html.in index 506c2669b4..24ee27cc5c 100644 --- a/docs/downloads.html.in +++ b/docs/downloads.html.in @@ -179,14 +179,14 @@ https://libvirt.org/sources/ruby/;>libvirt -https://gitlab.com/libvirt/ruby-libvirt;>gitlab +https://gitlab.com/libvirt/libvirt-ruby;>gitlab -https://gitlab.com/libvirt/ruby-libvirt/-/issues;>issues +https://gitlab.com/libvirt/libvirt-ruby/-/issues;>issues -https://libvirt.org/git/?p=ruby-libvirt.git;a=summary;>libvirt -https://github.com/libvirt/ruby-libvirt;>github +https://libvirt.org/git/?p=libvirt-ruby.git;a=summary;>libvirt +https://github.com/libvirt/libvirt-ruby;>github -- 2.25.4
Re: [libvirt-ruby PATCH v2 2/4] gitlab: introduce CI jobs testing git master & distro libvirt
On Fri, 2020-05-15 at 11:11 +0100, Daniel P. Berrangé wrote: > The ruby build needs to validate two axis > > - A variety of libvirt versions > - A variety of ruby versions > > We get coverage for both these axis by running a build against the > distro provided libvirt packages. All that is then missing is a build > against the latest libvirt git master, which only needs to be run on > a single distro, for which CentOS 8 is picked as a stable long life > base. > > For unknown reasons "rake package" fails on OpenSUSE Leap 15, but it > appears to be a bug in the old version of rake, so we skip that part > of the build. > > Signed-off-by: Daniel P. Berrangé > --- > .gitlab-ci.yml | 169 +++ > ci/README.rst| 14 +++ > ci/libvirt-centos-7.Dockerfile | 86 ++ > ci/libvirt-centos-8.Dockerfile | 64 ++ > ci/libvirt-debian-10.Dockerfile | 56 + > ci/libvirt-debian-9.Dockerfile | 59 ++ > ci/libvirt-debian-sid.Dockerfile | 56 + > ci/libvirt-fedora-31.Dockerfile | 53 + > ci/libvirt-fedora-32.Dockerfile | 53 + > ci/libvirt-fedora-rawhide.Dockerfile | 54 + > ci/libvirt-opensuse-151.Dockerfile | 55 + > ci/libvirt-ubuntu-1804.Dockerfile| 59 ++ > ci/libvirt-ubuntu-2004.Dockerfile| 56 + > ci/refresh | 27 + > 14 files changed, 861 insertions(+) > create mode 100644 ci/README.rst > create mode 100644 ci/libvirt-centos-7.Dockerfile > create mode 100644 ci/libvirt-centos-8.Dockerfile > create mode 100644 ci/libvirt-debian-10.Dockerfile > create mode 100644 ci/libvirt-debian-9.Dockerfile > create mode 100644 ci/libvirt-debian-sid.Dockerfile > create mode 100644 ci/libvirt-fedora-31.Dockerfile > create mode 100644 ci/libvirt-fedora-32.Dockerfile > create mode 100644 ci/libvirt-fedora-rawhide.Dockerfile > create mode 100644 ci/libvirt-opensuse-151.Dockerfile > create mode 100644 ci/libvirt-ubuntu-1804.Dockerfile > create mode 100644 ci/libvirt-ubuntu-2004.Dockerfile > create mode 100755 ci/refresh Reviewed-by: Andrea Bolognani -- Andrea Bolognani / Red Hat / Virtualization
Re: [libvirt-ruby PATCH v2 3/4] gitlab: add CONTRIBUTING.rst file to indicate use of merge requests
On Fri, 2020-05-15 at 11:11 +0100, Daniel P. Berrangé wrote: > With the introduction of automated CI pipelines, we are now ready to switch > to using merge requests for the project. With this switch we longer wish > to have patches sent to the mailing list. > > Signed-off-by: Daniel P. Berrangé > --- > .gitpublish | 4 > CONTRIBUTING.rst | 28 > 2 files changed, 28 insertions(+), 4 deletions(-) > delete mode 100644 .gitpublish > create mode 100644 CONTRIBUTING.rst Reviewed-by: Andrea Bolognani -- Andrea Bolognani / Red Hat / Virtualization
Re: [libvirt-ruby PATCH v2 1/4] doc: update for new git repo name and hosting
On Fri, 2020-05-15 at 11:11 +0100, Daniel P. Berrangé wrote: > +++ b/doc/site/index.html > @@ -61,7 +61,7 @@ puts dom.xml_desc > more complete examples. > Developers > The sources are available from git. To clone, run > -git clone git://libvirt.org/ruby-libvirt.git > +git clone https://gitlab.com/libvirt/libvirt-ruby Missing .git at the end of the clone URL. With that fixed, Reviewed-by: Andrea Bolognani -- Andrea Bolognani / Red Hat / Virtualization
Re: [libvirt-java PATCH v2 0/2] Introduce GitLab CI and merge requests
On Fri, 2020-05-15 at 11:30 +0100, Daniel P. Berrangé wrote: > In v2: > > - Use different approach for conditional job rules > - Use ubuntu for long term git job > > Daniel P. Berrangé (2): > gitlab: introduce CI jobs testing git master & distro libvirt > gitlab: add CONTRIBUTING.rst file to indicate use of merge requests Reviewed-by: Andrea Bolognani -- Andrea Bolognani / Red Hat / Virtualization
Re: [PATCH 21/40] datatypes: convert virNetwork to GObject
On Wed, May 13, 2020 at 01:57:05PM +0200, Rafael Fonseca wrote: > Signed-off-by: Rafael Fonseca > --- > src/conf/domain_conf.c | 6 ++-- > src/conf/network_event.c| 7 ++-- > src/conf/virnetworkobj.c| 5 +-- > src/datatypes.c | 50 + > src/datatypes.h | 19 +-- > src/libvirt-network.c | 7 ++-- > src/libvirt_private.syms| 2 +- > src/libxl/libxl_conf.c | 3 +- > src/libxl/xen_common.c | 3 +- > src/remote/remote_daemon_dispatch.c | 11 +++ > src/remote/remote_driver.c | 8 ++--- > src/rpc/gendispatch.pl | 1 + > 12 files changed, 59 insertions(+), 63 deletions(-) > > diff --git a/src/libxl/xen_common.c b/src/libxl/xen_common.c > index 5c37e431eb..631c9fe8d5 100644 > --- a/src/libxl/xen_common.c > +++ b/src/libxl/xen_common.c Needs to include datatypes.h otherwise build fails. PResumably a later patch in the series adds it, but too late. 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-ci PATCH] guests: don't force user to create a config.yaml file
On Fri, 2020-05-15 at 11:06 +0100, Daniel P. Berrangé wrote: > Only the "install" command requires a per-user config.yaml with a > password set. We should not require this for the other commands > to be run. This is incorrect: the update and build actions, and in general any action that requires using an Ansible playbook, needs it as well. I agree, however, that the hosts, projects and dockerfile actions should work even if the configuration file is not present. -- Andrea Bolognani / Red Hat / Virtualization
Re: [PATCH] README: Replace "configure" with "autogen.sh" for correct installation
On Fri, May 15, 2020 at 12:03:01PM +0200, Michal Privoznik wrote: > On 5/15/20 11:13 AM, Daniel P. Berrangé wrote: > > Or just ignore the problem for now, since when we switch to Meson, we'll > > have the same instructions regardless of whether building from git or > > tarballs. > > > > I'm not strictly against that, but I guess that might take a while. Pavel? > And this is clearly a feedback from somebody who is not a core developer, > that our build instructions are not as clear as they might be. It will take few more weeks I guess. I'm just finishing the src/ directory, the following once will not take that long, hopefully. Improving current documentation makes sense as the meson rewrite will just replace most of the commands and possible removes/adds a new once. Pavel signature.asc Description: PGP signature
Re: [PATCH 02/40] conf: capabilities: convert virCaps to GOBject
On Wed, May 13, 2020 at 01:56:46PM +0200, Rafael Fonseca wrote:
> Signed-off-by: Rafael Fonseca
> ---
> src/bhyve/bhyve_capabilities.c | 16
> src/bhyve/bhyve_driver.c| 29 --
> src/conf/capabilities.c | 39 ++---
> src/conf/capabilities.h | 6 ++---
> src/conf/storage_capabilities.c | 4 +--
> src/conf/virconftypes.h | 3 +--
> src/esx/esx_driver.c| 24 ++
> src/libvirt_private.syms| 1 +
> src/libxl/libxl_capabilities.c | 19 ++
> src/libxl/libxl_conf.c | 2 +-
> src/lxc/lxc_conf.c | 15 +--
> src/lxc/lxc_controller.c| 2 +-
> src/lxc/lxc_driver.c| 44 +++--
> src/lxc/lxc_process.c | 2 +-
> src/openvz/openvz_conf.c| 6 ++---
> src/qemu/qemu_capabilities.c| 14 +++
> src/qemu/qemu_conf.c| 6 ++---
> src/qemu/qemu_driver.c | 2 +-
> src/security/virt-aa-helper.c | 7 ++
> src/storage/storage_backend.c | 3 +--
> src/test/test_driver.c | 27 +---
> src/vbox/vbox_common.c | 6 ++---
> src/vmware/vmware_conf.c| 11 +++--
> src/vz/vz_driver.c | 26 ---
> tests/bhyveargv2xmltest.c | 2 +-
> tests/bhyvexml2argvtest.c | 2 +-
> tests/bhyvexml2xmltest.c| 2 +-
> tests/domainconftest.c | 2 +-
> tests/genericxml2xmltest.c | 2 +-
> tests/openvzutilstest.c | 2 +-
> tests/qemucaps2xmltest.c| 13 +++---
> tests/qemuhotplugtest.c | 6 ++---
> tests/testutils.c | 4 +--
> tests/testutilslxc.c| 20 ++-
> tests/testutilsqemu.c | 10 +++-
> tests/testutilsxen.c| 3 +--
> tests/vircaps2xmltest.c | 6 +
> tests/vircapstest.c | 14 +++
> tests/virresctrltest.c | 3 +--
> tests/vmx2xmltest.c | 8 ++
> tests/xml2vmxtest.c | 8 ++
> 41 files changed, 156 insertions(+), 265 deletions(-)
> diff --git a/tests/testutilsxen.c b/tests/testutilsxen.c
> index d50c3003da..7da90cdd1e 100644
> --- a/tests/testutilsxen.c
> +++ b/tests/testutilsxen.c
> @@ -11,7 +11,7 @@
> static virCapsPtr
> testXLInitCaps(void)
> {
> -virCapsPtr caps;
> +g_autoptr(virCaps) caps = NULL;
> virCapsGuestPtr guest;
> virCapsGuestMachinePtr *machines;
> int nmachines;
> @@ -78,7 +78,6 @@ testXLInitCaps(void)
>
> cleanup:
> virCapabilitiesFreeMachines(machines, nmachines);
> -virObjectUnref(caps);
> return NULL;
> }
Needs
diff --git a/tests/testutilsxen.c b/tests/testutilsxen.c
index 7da90cdd1e..80284c5b49 100644
--- a/tests/testutilsxen.c
+++ b/tests/testutilsxen.c
@@ -74,7 +74,7 @@ testXLInitCaps(void)
if (virCapabilitiesAddGuestDomain(guest, VIR_DOMAIN_VIRT_XEN, NULL,
NULL, 0, NULL) == NULL)
goto cleanup;
-return caps;
+return g_steal_pointer();
cleanup:
virCapabilitiesFreeMachines(machines, nmachines);
To fix the test suite crash
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 :|
[libvirt PATCH] cpu_arm: Drop unused variable
Signed-off-by: Jiri Denemark
---
Pushed. It is trivial and should fix the build on FreeBSD.
src/cpu/cpu_arm.c | 1 -
1 file changed, 1 deletion(-)
diff --git a/src/cpu/cpu_arm.c b/src/cpu/cpu_arm.c
index 66f6942ab9..6f6c6a1479 100644
--- a/src/cpu/cpu_arm.c
+++ b/src/cpu/cpu_arm.c
@@ -325,7 +325,6 @@ virCPUarmModelParse(xmlXPathContextPtr ctxt,
{
virCPUarmMapPtr map = data;
g_autoptr(virCPUarmModel) model = NULL;
-g_autofree xmlNodePtr *nodes = NULL;
g_autofree char *vendor = NULL;
model = g_new0(virCPUarmModel, 1);
--
2.26.2
Re: [PATCH 00/40] convert virObjects to GObject
On 5/13/20 1:56 PM, Rafael Fonseca wrote: This patch series convert various simple instances of virObject to a GObject equivalent. I'm sorry upfront for misusing your patchset and I'm also sorry for bringing this up again. I think we need to step back and re-evaluate whether this is worth it. GObject is horrible and frightening part of GLib. Not only one has to define empty functions, but we can't mix virObject and GObject. For instance: virObjectRef() called over GObject will corrupt memory. Worse, there is no way to check whether your patches converted everything (and clearly they did not because I see couple of tests crashing; or maybe they did at the time of send but now the code has changed - anyway, point proven). I started reviewing and stopped at the first patch realizing, I have no idea whether you converted every virObjectRef()/virObjectUnref() with corresponding glib call. I also wanted to write a cocci spatch that might at least identify places where that is happening, but apparently my spatch skills are poor. Does anybody have an idea how to verify these patches? Michal
Re: [libvirt PATCH] docs: document proper enum for guest agent timeout
On 5/15/20 12:30 PM, Tomáš Golembiovský wrote: The documented enum and its values do not exits. The real enum has slightly different name. Signed-off-by: Tomáš Golembiovský --- src/libvirt-domain.c | 8 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/libvirt-domain.c b/src/libvirt-domain.c index a12809c2d5..37f864b7b0 100644 --- a/src/libvirt-domain.c +++ b/src/libvirt-domain.c @@ -12550,13 +12550,13 @@ int virDomainGetLaunchSecurityInfo(virDomainPtr domain, * Set how long to wait for a response from guest agent commands. By default, * agent commands block forever waiting for a response. * - * @timeout must be a value from virDomainAgentCommandTimeoutValues or + * @timeout must be a value from virDomainAgentResponseTimeoutValues or * positive: * - * VIR_DOMAIN_AGENT_COMMAND_TIMEOUT_BLOCK(-2): meaning to block forever + * VIR_DOMAIN_AGENT_RESPONSE_TIMEOUT_BLOCK(-2): meaning to block forever * waiting for a result. - * VIR_DOMAIN_AGENT_COMMAND_TIMEOUT_DEFAULT(-1): use default timeout value. - * VIR_DOMAIN_AGENT_COMMAND_TIMEOUT_NOWAIT(0): does not wait. + * VIR_DOMAIN_AGENT_RESPONSE_TIMEOUT_DEFAULT(-1): use default timeout value. + * VIR_DOMAIN_AGENT_RESPONSE_TIMEOUT_NOWAIT(0): does not wait. * positive value: wait for @timeout seconds * * Returns 0 on success, -1 on failure Oops. Reviewed-by: Michal Privoznik Pushed. Michal
Re: [libvirt PATCH] qemu: reject readonly attribute for virtiofs
On Wed, May 13, 2020 at 01:06:58PM +0200, Peter Krempa wrote: > On Wed, May 13, 2020 at 11:20:07 +0100, Daniel Berrange wrote: > > On Wed, May 13, 2020 at 12:05:44PM +0200, Peter Krempa wrote: > > > On Wed, May 13, 2020 at 10:57:33 +0100, Daniel Berrange wrote: > > > > On Wed, May 13, 2020 at 11:51:50AM +0200, Peter Krempa wrote: > > [...] > > > > > The point is for libvirt to follow normal practice from GitLab, so that > > > > contributors don't have to know about libvirt specific rules for > > > > contributing > > > > to the project. Telling users to change the normal issue syntax into a > > > > URL > > > > whenever they send a patch is not something we should be doing. > > > > > > Even if they are flawed?! Just mentioning a random number in the commit > > > message may be nice for people looking at the code via web-ui. That is > > > nice but you can't do much there. But I and many other look at the code > > > in the local checkout and 'git' doesn't randomly decide to expand the > > > number to the gitlab uri and make it clickable. > > > > > > Now if I'd like to look at the issue after some time I'll e.g. when > > > going through git-blame I'll have to open the browser, open gitlab, find > > > the project and find the issue rather than just click a link. That's > > > stupid. If gitlab can't deal with the link, it's frankly broken and we > > > should not give in to a broken approach just because everybody else > > > does. > > > > It isn't about giving in. Again the point is to not needlessly create > > special rules for contributing to libvirt, because every special rule > > we add is another thing for contributors to stumble over. Some rules > > are worth it because they have meaningful benefits such as the use of > > Signed-off-by/DCO. The mentioning of full URLs instead of the normal > > issue reference syntax does not have a meaningful benefit that > > justifies a libvirt special rule for contributions. > > I gave an examples of two specific meaningful benefit above: > > 1) it provides a clickable link without second guessing where to go for > command line users > 2) provides stable reference to the hosting of issues > > Note that for example github uses exactly the same format for > referencing issues. That means that it's unclear what we are referring. > > Just observe the following behaviours on the commit that was just pushed > and Jano helpfully for this demonstration included both versions: > > 1) gitlab: libvirt/libvirt > > https://gitlab.com/libvirt/libvirt/-/commit/9c58b6eb0090b978f44299b22ff4f8d3be3a240f > > Here I'd expect a clickable link but there isn't one. I have no idea > why. > > You can see that gitlab doesn't recognize it. I'm not sure whether the > format Jano chose is bad, but this already enforces some kind of form we > need to conform to. > > 2) gitlab: pipo.sk/libvirt > > https://gitlab.com/pipo.sk/libvirt/-/commit/9c58b6eb0090b978f44299b22ff4f8d3be3a240f > > Unsurprisingly it doesn't work here, but the full link is shortened and > still points to the correct issue. > > 3) github: libvirt/libvirt > > https://github.com/libvirt/libvirt/commit/9c58b6eb0090b978f44299b22ff4f8d3be3a240f > > As expected #23 points to nothing, but the full link still points to the > issue even cross-hosting. > > 4) github: pipo/libvirt (plus I've created 23 test issues in my fork) > > https://gitlab.com/pipo.sk/libvirt/-/commit/9c58b6eb0090b978f44299b22ff4f8d3be3a240f > > Now this is fun. '#23' points to the local issue #23 in my fork while > the full link still points to the issue. > > > The shortened issue names are ambiguous and the hosting has no way in > figuring out where to point to. Providing full URL is not something > which should be described as "no meaningful benefit" but it actively > disambiguates the links regardless of where it's hosted or refered from. Ok, I'll withdraw my objection to use of full URLs. 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] README: Replace "configure" with "autogen.sh" for correct installation
On Fri, May 15, 2020 at 12:03:01PM +0200, Michal Privoznik wrote: > On 5/15/20 11:13 AM, Daniel P. Berrangé wrote: > > Or just ignore the problem for now, since when we switch to Meson, we'll > > have the same instructions regardless of whether building from git or > > tarballs. > > > > I'm not strictly against that, but I guess that might take a while. Pavel? > And this is clearly a feedback from somebody who is not a core developer, > that our build instructions are not as clear as they might be. True, lets just remove the instructions from README.rst, and add a line that says "For instructions on building libvirt see docs/compiling.rst" and convert compiling.html.in to RST so that it is pleasant to read 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 :|
[libvirt-java PATCH v2 2/2] gitlab: add CONTRIBUTING.rst file to indicate use of merge requests
With the introduction of automated CI pipelines, we are now ready to switch to using merge requests for the project. With this switch we longer wish to have patches sent to the mailing list, and thus the git-publish config is removed. Reviewed-by: Andrea Bolognani Signed-off-by: Daniel P. Berrangé --- .gitpublish | 4 CONTRIBUTING.rst | 28 2 files changed, 28 insertions(+), 4 deletions(-) delete mode 100644 .gitpublish create mode 100644 CONTRIBUTING.rst diff --git a/.gitpublish b/.gitpublish deleted file mode 100644 index 1920230..000 --- a/.gitpublish +++ /dev/null @@ -1,4 +0,0 @@ -[gitpublishprofile "default"] -base = master -to = libvir-list@redhat.com -prefix = libvirt-java PATCH diff --git a/CONTRIBUTING.rst b/CONTRIBUTING.rst new file mode 100644 index 000..4ef23bd --- /dev/null +++ b/CONTRIBUTING.rst @@ -0,0 +1,28 @@ + +Contributing to libvirt-java + + +The libvirt Java API binding accepts code contributions via merge requests +on the GitLab project: + +https://gitlab.com/libvirt/libvirt-java/-/merge_requests + +It is required that automated CI pipelines succeed before a merge request +will be accepted. The global pipeline status for the ``master`` branch is +visible at: + +https://gitlab.com/libvirt/libvirt-java/pipelines + +CI pipeline results for merge requests will be visible via the contributors' +own private repository fork: + +https://gitlab.com/yourusername/libvirt-java/pipelines + +Contributions submitted to the project must be in compliance with the +Developer Certificate of Origin Version 1.1. This is documented at: + +https://developercertificate.org/ + +To indicate compliance, each commit in a series must have a "Signed-off-by" +tag with the submitter's name and email address. This can be added by passing +the ``-s`` flag to ``git commit`` when creating the patches. -- 2.26.2
[libvirt-java PATCH v2 1/2] gitlab: introduce CI jobs testing git master & distro libvirt
The java build needs to validate two axis - A variety of libvirt versions - A variety of java versions We get coverage for both these axis by running a build against the distro provided libvirt packages. All that is then missing is a build against the latest libvirt git master, which only needs to be run on a single distro, for which Fedora 32 is picked. CentOS 8 is unable to run tests, since it lacks ant-junit packages. Signed-off-by: Daniel P. Berrangé --- .gitlab-ci.yml | 171 +++ ci/README.rst| 14 +++ ci/libvirt-centos-7.Dockerfile | 88 ++ ci/libvirt-centos-8.Dockerfile | 55 + ci/libvirt-debian-10.Dockerfile | 58 + ci/libvirt-debian-9.Dockerfile | 61 ++ ci/libvirt-debian-sid.Dockerfile | 58 + ci/libvirt-fedora-31.Dockerfile | 55 + ci/libvirt-fedora-32.Dockerfile | 55 + ci/libvirt-fedora-rawhide.Dockerfile | 56 + ci/libvirt-opensuse-151.Dockerfile | 57 + ci/libvirt-ubuntu-1804.Dockerfile| 61 ++ ci/libvirt-ubuntu-2004.Dockerfile| 68 +++ ci/refresh | 27 + 14 files changed, 884 insertions(+) create mode 100644 ci/README.rst create mode 100644 ci/libvirt-centos-7.Dockerfile create mode 100644 ci/libvirt-centos-8.Dockerfile create mode 100644 ci/libvirt-debian-10.Dockerfile create mode 100644 ci/libvirt-debian-9.Dockerfile create mode 100644 ci/libvirt-debian-sid.Dockerfile create mode 100644 ci/libvirt-fedora-31.Dockerfile create mode 100644 ci/libvirt-fedora-32.Dockerfile create mode 100644 ci/libvirt-fedora-rawhide.Dockerfile create mode 100644 ci/libvirt-opensuse-151.Dockerfile create mode 100644 ci/libvirt-ubuntu-1804.Dockerfile create mode 100644 ci/libvirt-ubuntu-2004.Dockerfile create mode 100755 ci/refresh diff --git a/.gitlab-ci.yml b/.gitlab-ci.yml index 50dae92..cb11858 100644 --- a/.gitlab-ci.yml +++ b/.gitlab-ci.yml @@ -1,6 +1,59 @@ stages: - prebuild + - containers + - builds + +.container_job_template: _job_definition + image: docker:stable + stage: containers + services: +- docker:dind + before_script: +- export TAG="$CI_REGISTRY_IMAGE/ci-$NAME:latest" +- export COMMON_TAG="$CI_REGISTRY/libvirt/libvirt-java/ci-$NAME:latest" +- docker info +- docker login registry.gitlab.com -u "$CI_REGISTRY_USER" -p "$CI_REGISTRY_PASSWORD" + script: +- docker pull "$TAG" || docker pull "$COMMON_TAG" || true +- docker build --cache-from "$TAG" --cache-from "$COMMON_TAG" --tag "$TAG" -f "ci/libvirt-$NAME.Dockerfile" ci +- docker push "$TAG" + after_script: +- docker logout + +.git_build_job_template: _build_job_definition + image: $CI_REGISTRY_IMAGE/ci-$NAME:latest + stage: builds + before_script: +- export MAKEFLAGS="-j$(getconf _NPROCESSORS_ONLN)" +- export SCRATCH_DIR="/tmp/scratch" +- export VROOT="$SCRATCH_DIR/vroot" +- export LD_LIBRARY_PATH="$VROOT/lib" +- export PATH="$VROOT/bin:$PATH" +- export PKG_CONFIG_PATH="$VROOT/lib/pkgconfig" + script: +- pushd "$PWD" +- mkdir -p "$SCRATCH_DIR" +- cd "$SCRATCH_DIR" +- git clone --depth 1 https://gitlab.com/libvirt/libvirt.git +- mkdir libvirt/build +- cd libvirt/build +- ../autogen.sh --prefix="$VROOT" --without-libvirtd +- $MAKE install +- popd +- ant build jar docs +- if test "$TESTS" != "skip" ; then ant test ; fi +- ant src +- if test -x /usr/bin/rpmbuild ; then ant spec && rpmbuild -ba --define "_sourcedir `pwd`/target" target/libvirt-java.spec ; fi + +.dist_build_job_template: _build_job_definition + image: $CI_REGISTRY_IMAGE/ci-$NAME:latest + stage: builds + script: +- ant build jar docs +- if test "$TESTS" != "skip" ; then ant test ; fi +- ant src +- if test -x /usr/bin/rpmbuild ; then ant spec && rpmbuild -ba --define "_sourcedir `pwd`/target" target/libvirt-java.spec ; fi # Check that all commits are signed-off for the DCO. # Skip on "libvirt" namespace, since we only need to run @@ -14,3 +67,121 @@ check-dco: except: variables: - $CI_PROJECT_NAMESPACE == 'libvirt' + +centos-7-container: + <<: *container_job_definition + variables: +NAME: centos-7 + +centos-8-container: + <<: *container_job_definition + variables: +NAME: centos-8 + +debian-9-container: + <<: *container_job_definition + variables: +NAME: debian-9 + +debian-10-container: + <<: *container_job_definition + variables: +NAME: debian-10 + +debian-sid-container: + <<: *container_job_definition + variables: +NAME: debian-sid + +fedora-31-container: + <<: *container_job_definition + variables: +NAME: fedora-31 + +fedora-32-container: + <<: *container_job_definition + variables: +NAME: fedora-32 + +fedora-rawhide-container: + <<: *container_job_definition + variables: +NAME: fedora-rawhide +
[libvirt-java PATCH v2 0/2] Introduce GitLab CI and merge requests
In v2: - Use different approach for conditional job rules - Use ubuntu for long term git job Daniel P. Berrangé (2): gitlab: introduce CI jobs testing git master & distro libvirt gitlab: add CONTRIBUTING.rst file to indicate use of merge requests .gitlab-ci.yml | 171 +++ .gitpublish | 4 - CONTRIBUTING.rst | 28 + ci/README.rst| 14 +++ ci/libvirt-centos-7.Dockerfile | 88 ++ ci/libvirt-centos-8.Dockerfile | 55 + ci/libvirt-debian-10.Dockerfile | 58 + ci/libvirt-debian-9.Dockerfile | 61 ++ ci/libvirt-debian-sid.Dockerfile | 58 + ci/libvirt-fedora-31.Dockerfile | 55 + ci/libvirt-fedora-32.Dockerfile | 55 + ci/libvirt-fedora-rawhide.Dockerfile | 56 + ci/libvirt-opensuse-151.Dockerfile | 57 + ci/libvirt-ubuntu-1804.Dockerfile| 61 ++ ci/libvirt-ubuntu-2004.Dockerfile| 68 +++ ci/refresh | 27 + 16 files changed, 912 insertions(+), 4 deletions(-) delete mode 100644 .gitpublish create mode 100644 CONTRIBUTING.rst create mode 100644 ci/README.rst create mode 100644 ci/libvirt-centos-7.Dockerfile create mode 100644 ci/libvirt-centos-8.Dockerfile create mode 100644 ci/libvirt-debian-10.Dockerfile create mode 100644 ci/libvirt-debian-9.Dockerfile create mode 100644 ci/libvirt-debian-sid.Dockerfile create mode 100644 ci/libvirt-fedora-31.Dockerfile create mode 100644 ci/libvirt-fedora-32.Dockerfile create mode 100644 ci/libvirt-fedora-rawhide.Dockerfile create mode 100644 ci/libvirt-opensuse-151.Dockerfile create mode 100644 ci/libvirt-ubuntu-1804.Dockerfile create mode 100644 ci/libvirt-ubuntu-2004.Dockerfile create mode 100755 ci/refresh -- 2.26.2
[libvirt PATCH] docs: document proper enum for guest agent timeout
The documented enum and its values do not exits. The real enum has slightly different name. Signed-off-by: Tomáš Golembiovský --- src/libvirt-domain.c | 8 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/libvirt-domain.c b/src/libvirt-domain.c index a12809c2d5..37f864b7b0 100644 --- a/src/libvirt-domain.c +++ b/src/libvirt-domain.c @@ -12550,13 +12550,13 @@ int virDomainGetLaunchSecurityInfo(virDomainPtr domain, * Set how long to wait for a response from guest agent commands. By default, * agent commands block forever waiting for a response. * - * @timeout must be a value from virDomainAgentCommandTimeoutValues or + * @timeout must be a value from virDomainAgentResponseTimeoutValues or * positive: * - * VIR_DOMAIN_AGENT_COMMAND_TIMEOUT_BLOCK(-2): meaning to block forever + * VIR_DOMAIN_AGENT_RESPONSE_TIMEOUT_BLOCK(-2): meaning to block forever * waiting for a result. - * VIR_DOMAIN_AGENT_COMMAND_TIMEOUT_DEFAULT(-1): use default timeout value. - * VIR_DOMAIN_AGENT_COMMAND_TIMEOUT_NOWAIT(0): does not wait. + * VIR_DOMAIN_AGENT_RESPONSE_TIMEOUT_DEFAULT(-1): use default timeout value. + * VIR_DOMAIN_AGENT_RESPONSE_TIMEOUT_NOWAIT(0): does not wait. * positive value: wait for @timeout seconds * * Returns 0 on success, -1 on failure -- 2.25.0
[libvirt-ruby PATCH v2 1/4] doc: update for new git repo name and hosting
Signed-off-by: Daniel P. Berrangé --- doc/site/downloads.html | 12 ++-- doc/site/index.html | 2 +- 2 files changed, 3 insertions(+), 11 deletions(-) diff --git a/doc/site/downloads.html b/doc/site/downloads.html index cc656c9..41964f7 100644 --- a/doc/site/downloads.html +++ b/doc/site/downloads.html @@ -64,18 +64,10 @@ GIT source repository The ruby-libvirt source code that is in development is maintained in a http://git-scm.com/;>git repository available on - http://libvirt.org/git/;>libvirt.org: + http://gitlab.com/libvirt/libvirt-ruby;>GitLab: - -git clone git://libvirt.org/ruby-libvirt.git - - - It can also be browsed at: - - - -http://libvirt.org/git/?p=ruby-libvirt.git;a=summary;>http://libvirt.org/git/?p=ruby-libvirt.git;a=summary +git clone https://gitlab.com/libvirt/libvirt-ruby.git Compilation In order to compile ruby-libvirt, you will need to have the diff --git a/doc/site/index.html b/doc/site/index.html index b421c5e..8001e4b 100644 --- a/doc/site/index.html +++ b/doc/site/index.html @@ -61,7 +61,7 @@ puts dom.xml_desc more complete examples. Developers The sources are available from git. To clone, run -git clone git://libvirt.org/ruby-libvirt.git +git clone https://gitlab.com/libvirt/libvirt-ruby Please use the https://www.redhat.com/mailman/listinfo/libvir-list/;>libvirt mailing list for questions, comments and patches. -- 2.26.2
[libvirt-ruby PATCH v2 4/4] Remove obsolete mercurial ignore file
Reviewed-by: Andrea Bolognani Signed-off-by: Daniel P. Berrangé --- .hgignore | 7 --- 1 file changed, 7 deletions(-) delete mode 100644 .hgignore diff --git a/.hgignore b/.hgignore deleted file mode 100644 index 8158c7f..000 --- a/.hgignore +++ /dev/null @@ -1,7 +0,0 @@ -~$ -.o$ -.so$ -Makefile$ -^pkg/ -^ext/libvirt/mkmf.log$ -^doc/site/api -- 2.26.2
[libvirt-ruby PATCH v2 3/4] gitlab: add CONTRIBUTING.rst file to indicate use of merge requests
With the introduction of automated CI pipelines, we are now ready to switch to using merge requests for the project. With this switch we longer wish to have patches sent to the mailing list. Signed-off-by: Daniel P. Berrangé --- .gitpublish | 4 CONTRIBUTING.rst | 28 2 files changed, 28 insertions(+), 4 deletions(-) delete mode 100644 .gitpublish create mode 100644 CONTRIBUTING.rst diff --git a/.gitpublish b/.gitpublish deleted file mode 100644 index 8e6d748..000 --- a/.gitpublish +++ /dev/null @@ -1,4 +0,0 @@ -[gitpublishprofile "default"] -base = master -to = libvir-list@redhat.com -prefix = ruby PATCH diff --git a/CONTRIBUTING.rst b/CONTRIBUTING.rst new file mode 100644 index 000..b145062 --- /dev/null +++ b/CONTRIBUTING.rst @@ -0,0 +1,28 @@ + +Contributing to libvirt-ruby + + +The libvirt Ruby API binding accepts code contributions via merge requests +on the GitLab project: + +https://gitlab.com/libvirt/libvirt-ruby/-/merge_requests + +It is required that automated CI pipelines succeed before a merge request +will be accepted. The global pipeline status for the ``master`` branch is +visible at: + +https://gitlab.com/libvirt/libvirt-ruby/pipelines + +CI pipeline results for merge requests will be visible via the contributors' +own private repository fork: + +https://gitlab.com/yourusername/libvirt-ruby/pipelines + +Contributions submitted to the project must be in compliance with the +Developer Certificate of Origin Version 1.1. This is documented at: + +https://developercertificate.org/ + +To indicate compliance, each commit in a series must have a "Signed-off-by" +tag with the submitter's name and email address. This can be added by passing +the ``-s`` flag to ``git commit`` when creating the patches. -- 2.26.2
[libvirt-ruby PATCH v2 0/4] Introduce GitLab CI and merge requests
In v2: - Renamed repo - Change way we conditionalize build rules Daniel P. Berrangé (4): doc: update for new git repo name and hosting gitlab: introduce CI jobs testing git master & distro libvirt gitlab: add CONTRIBUTING.rst file to indicate use of merge requests Remove obsolete mercurial ignore file .gitlab-ci.yml | 169 +++ .gitpublish | 4 - .hgignore| 7 -- CONTRIBUTING.rst | 28 + ci/README.rst| 14 +++ ci/libvirt-centos-7.Dockerfile | 86 ++ ci/libvirt-centos-8.Dockerfile | 64 ++ ci/libvirt-debian-10.Dockerfile | 56 + ci/libvirt-debian-9.Dockerfile | 59 ++ ci/libvirt-debian-sid.Dockerfile | 56 + ci/libvirt-fedora-31.Dockerfile | 53 + ci/libvirt-fedora-32.Dockerfile | 53 + ci/libvirt-fedora-rawhide.Dockerfile | 54 + ci/libvirt-opensuse-151.Dockerfile | 55 + ci/libvirt-ubuntu-1804.Dockerfile| 59 ++ ci/libvirt-ubuntu-2004.Dockerfile| 56 + ci/refresh | 27 + doc/site/downloads.html | 12 +- doc/site/index.html | 2 +- 19 files changed, 892 insertions(+), 22 deletions(-) delete mode 100644 .gitpublish delete mode 100644 .hgignore create mode 100644 CONTRIBUTING.rst create mode 100644 ci/README.rst create mode 100644 ci/libvirt-centos-7.Dockerfile create mode 100644 ci/libvirt-centos-8.Dockerfile create mode 100644 ci/libvirt-debian-10.Dockerfile create mode 100644 ci/libvirt-debian-9.Dockerfile create mode 100644 ci/libvirt-debian-sid.Dockerfile create mode 100644 ci/libvirt-fedora-31.Dockerfile create mode 100644 ci/libvirt-fedora-32.Dockerfile create mode 100644 ci/libvirt-fedora-rawhide.Dockerfile create mode 100644 ci/libvirt-opensuse-151.Dockerfile create mode 100644 ci/libvirt-ubuntu-1804.Dockerfile create mode 100644 ci/libvirt-ubuntu-2004.Dockerfile create mode 100755 ci/refresh -- 2.26.2
[libvirt-ruby PATCH v2 2/4] gitlab: introduce CI jobs testing git master & distro libvirt
The ruby build needs to validate two axis - A variety of libvirt versions - A variety of ruby versions We get coverage for both these axis by running a build against the distro provided libvirt packages. All that is then missing is a build against the latest libvirt git master, which only needs to be run on a single distro, for which CentOS 8 is picked as a stable long life base. For unknown reasons "rake package" fails on OpenSUSE Leap 15, but it appears to be a bug in the old version of rake, so we skip that part of the build. Signed-off-by: Daniel P. Berrangé --- .gitlab-ci.yml | 169 +++ ci/README.rst| 14 +++ ci/libvirt-centos-7.Dockerfile | 86 ++ ci/libvirt-centos-8.Dockerfile | 64 ++ ci/libvirt-debian-10.Dockerfile | 56 + ci/libvirt-debian-9.Dockerfile | 59 ++ ci/libvirt-debian-sid.Dockerfile | 56 + ci/libvirt-fedora-31.Dockerfile | 53 + ci/libvirt-fedora-32.Dockerfile | 53 + ci/libvirt-fedora-rawhide.Dockerfile | 54 + ci/libvirt-opensuse-151.Dockerfile | 55 + ci/libvirt-ubuntu-1804.Dockerfile| 59 ++ ci/libvirt-ubuntu-2004.Dockerfile| 56 + ci/refresh | 27 + 14 files changed, 861 insertions(+) create mode 100644 ci/README.rst create mode 100644 ci/libvirt-centos-7.Dockerfile create mode 100644 ci/libvirt-centos-8.Dockerfile create mode 100644 ci/libvirt-debian-10.Dockerfile create mode 100644 ci/libvirt-debian-9.Dockerfile create mode 100644 ci/libvirt-debian-sid.Dockerfile create mode 100644 ci/libvirt-fedora-31.Dockerfile create mode 100644 ci/libvirt-fedora-32.Dockerfile create mode 100644 ci/libvirt-fedora-rawhide.Dockerfile create mode 100644 ci/libvirt-opensuse-151.Dockerfile create mode 100644 ci/libvirt-ubuntu-1804.Dockerfile create mode 100644 ci/libvirt-ubuntu-2004.Dockerfile create mode 100755 ci/refresh diff --git a/.gitlab-ci.yml b/.gitlab-ci.yml index 50dae92..3b19d82 100644 --- a/.gitlab-ci.yml +++ b/.gitlab-ci.yml @@ -1,6 +1,57 @@ stages: - prebuild + - containers + - builds + +.container_job_template: _job_definition + image: docker:stable + stage: containers + services: +- docker:dind + before_script: +- export TAG="$CI_REGISTRY_IMAGE/ci-$NAME:latest" +- export COMMON_TAG="$CI_REGISTRY/libvirt/libvirt-ruby/ci-$NAME:latest" +- docker info +- docker login registry.gitlab.com -u "$CI_REGISTRY_USER" -p "$CI_REGISTRY_PASSWORD" + script: +- docker pull "$TAG" || docker pull "$COMMON_TAG" || true +- docker build --cache-from "$TAG" --cache-from "$COMMON_TAG" --tag "$TAG" -f "ci/libvirt-$NAME.Dockerfile" ci +- docker push "$TAG" + after_script: +- docker logout + +.git_build_job_template: _build_job_definition + image: $CI_REGISTRY_IMAGE/ci-$NAME:latest + stage: builds + before_script: +- export MAKEFLAGS="-j$(getconf _NPROCESSORS_ONLN)" +- export SCRATCH_DIR="/tmp/scratch" +- export VROOT="$SCRATCH_DIR/vroot" +- export LD_LIBRARY_PATH="$VROOT/lib" +- export PATH="$VROOT/bin:$PATH" +- export PKG_CONFIG_PATH="$VROOT/lib/pkgconfig" + script: +- pushd "$PWD" +- mkdir -p "$SCRATCH_DIR" +- cd "$SCRATCH_DIR" +- git clone --depth 1 https://gitlab.com/libvirt/libvirt.git +- mkdir libvirt/build +- cd libvirt/build +- ../autogen.sh --prefix="$VROOT" --without-libvirtd +- $MAKE install +- popd +- rake build +- if test "$DIST" != "skip" ; then rake package; fi + +.dist_build_job_template: _build_job_definition + image: $CI_REGISTRY_IMAGE/ci-$NAME:latest + stage: builds + before_script: +- export MAKEFLAGS="-j$(getconf _NPROCESSORS_ONLN)" + script: +- rake build +- if test "$DIST" != "skip" ; then rake package; fi # Check that all commits are signed-off for the DCO. # Skip on "libvirt" namespace, since we only need to run @@ -14,3 +65,121 @@ check-dco: except: variables: - $CI_PROJECT_NAMESPACE == 'libvirt' + +centos-7-container: + <<: *container_job_definition + variables: +NAME: centos-7 + +centos-8-container: + <<: *container_job_definition + variables: +NAME: centos-8 + +debian-9-container: + <<: *container_job_definition + variables: +NAME: debian-9 + +debian-10-container: + <<: *container_job_definition + variables: +NAME: debian-10 + +debian-sid-container: + <<: *container_job_definition + variables: +NAME: debian-sid + +fedora-31-container: + <<: *container_job_definition + variables: +NAME: fedora-31 + +fedora-32-container: + <<: *container_job_definition + variables: +NAME: fedora-32 + +fedora-rawhide-container: + <<: *container_job_definition + variables: +NAME: fedora-rawhide + +opensuse-151-container: + <<: *container_job_definition + variables: +NAME: opensuse-151 + +ubuntu-1804-container: + <<:
[libvirt-ci PATCH] guests: don't force user to create a config.yaml file
Only the "install" command requires a per-user config.yaml with a
password set. We should not require this for the other commands
to be run.
Signed-off-by: Daniel P. Berrangé
---
guests/lcitool | 51 +-
1 file changed, 25 insertions(+), 26 deletions(-)
diff --git a/guests/lcitool b/guests/lcitool
index 27aee7b..1f093d0 100755
--- a/guests/lcitool
+++ b/guests/lcitool
@@ -138,11 +138,15 @@ class Config:
with open(os.path.join(base, "config.yaml"), "r") as fp:
self.values = yaml.safe_load(fp)
+user_config_path = self._get_config_file("config.yaml")
+if not os.path.exists(user_config_path):
+return
+
try:
-with open(self._get_config_file("config.yaml"), "r") as fp:
+with open(user_config_path, "r") as fp:
user_config = yaml.safe_load(fp)
except Exception as e:
-raise Exception("Missing or invalid config.yaml file:
{}".format(e))
+raise Exception("Invalid config.yaml file: {}".format(e))
if user_config is None:
raise Exception("Missing or invalid config.yaml file")
@@ -181,20 +185,13 @@ class Config:
if k not in known_keys:
del _dict[k]
-def _validate_section(self, config, section, mandatory_keys):
+def _validate_section(self, config, section):
# remove keys we don't recognize
self._remove_unknown_keys(config[section], self.values[section].keys())
-# check that the mandatory keys are present and non-empty
-for key in mandatory_keys:
-if config.get(section).get(key) is None:
-raise Exception(("Missing or empty value for mandatory key"
- "'{}.{}'").format(section, key))
-
# check that all keys have values assigned and of the right type
for key in config[section].keys():
-# mandatory keys were already checked, so this covers optional keys
if config[section][key] is None:
raise Exception(
"Missing value for '{}.{}'".format(section, key)
@@ -209,22 +206,6 @@ class Config:
# delete sections we don't recognize
self._remove_unknown_keys(config, self.values.keys())
-if "install" not in config:
-raise Exception("Missing mandatory section 'install'")
-
-self._validate_section(config, "install", ["root_password"])
-
-# we only need this for the gitlab check below, if 'flavor' is missing
-# that's okay, we'll provide a default later
-flavor = config["install"].get("flavor")
-if flavor is not None and flavor not in ["test", "jenkins", "gitlab"]:
-raise Exception(
-"Invalid value '{}' for 'install.flavor'".format(flavor)
-)
-
-if flavor == "gitlab":
-self._validate_section(config, "gitlab", ["runner_secret"])
-
def _update(self, values):
self.values["install"].update(values["install"])
@@ -561,10 +542,28 @@ class Application:
for project in self._projects.expand_pattern("all"):
print(project)
+def _validate_install_config(self, config):
+if config.values["install"].get("root_password") is None:
+raise Exception("Missing mandatory install.root_password
config.yaml parameter")
+
+# we only need this for the gitlab check below, if 'flavor' is missing
+# that's okay, we'll provide a default later
+flavor = config.values["install"].get("flavor")
+if flavor is not None and flavor not in ["test", "jenkins", "gitlab"]:
+raise Exception(
+"Invalid value '{}' for 'install.flavor'".format(flavor)
+)
+
+if flavor == "gitlab":
+if config.values["gitlab"].get("runner_secret") is None:
+raise Exception("Missing mandatory gitlab.runner_secret
config.yaml parameter")
+
def _action_install(self, args):
base = Util.get_base()
config = self._config
+self._validate_install_config(config)
+
for host in self._inventory.expand_pattern(args.hosts):
facts = self._inventory.get_facts(host)
--
2.26.2
Re: [PATCH] README: Replace "configure" with "autogen.sh" for correct installation
On 5/15/20 11:13 AM, Daniel P. Berrangé wrote: Or just ignore the problem for now, since when we switch to Meson, we'll have the same instructions regardless of whether building from git or tarballs. I'm not strictly against that, but I guess that might take a while. Pavel? And this is clearly a feedback from somebody who is not a core developer, that our build instructions are not as clear as they might be. Michal
Re: [PATCH V5 0/4] Introduce getHost support for ARM CPU driver
On Wed, May 13, 2020 at 18:48:28 +0800, Zhenyu Zheng wrote: > Introduce getHost support for ARM CPU driver. First add > some data about commonly used ARM CPU models, and their > vendors into cpu_map, then added some helper methods as > callbacks to load them. Read and parse vendor_id, part_id > and CPU flags of local CPU from corresponding registers. > > Signed-off-by: Zhenyu Zheng > > Zhenyu Zheng (4): > cpu: Introduce virCPUarmData and related struts > cpu: Add helper functions to parse vendor and model > cpu: Introduce getHost support for ARM CPU driver > cpu_map: Introduce ARM cpu models Nice, there's a few minor issues that needs fixing, but since I already did all of them and really checked the code compiles fine after each patch and tested it can still detect the host CPU, I'll squash the suggested changes and push the series. Reviewed-by: Jiri Denemark
Re: [PATCH V5 2/4] cpu: Add helper functions to parse vendor and model
On Wed, May 13, 2020 at 18:48:32 +0800, Zhenyu Zheng wrote:
> Add helper functions to parse vendor and model for
> ARM CPUs, and use them as callbacks when load cpu
> maps.
>
> Signed-off-by: Zhenyu Zheng
> ---
> src/cpu/cpu_arm.c | 159 +-
> 1 file changed, 158 insertions(+), 1 deletion(-)
>
> diff --git a/src/cpu/cpu_arm.c b/src/cpu/cpu_arm.c
> index 1bb0afb762..c7a6abfb22 100644
> --- a/src/cpu/cpu_arm.c
> +++ b/src/cpu/cpu_arm.c
> @@ -165,6 +165,7 @@ virCPUarmMapFree(virCPUarmMapPtr map)
> virCPUarmVendorFree(map->vendors[i]);
> g_free(map->vendors);
>
> +
> g_ptr_array_free(map->features, TRUE);
>
> g_free(map);
Unintentional change IÂ guess.
> @@ -210,6 +211,160 @@ virCPUarmMapFeatureParse(xmlXPathContextPtr ctxt
> G_GNUC_UNUSED,
> return 0;
> }
>
> +static virCPUarmVendorPtr
> +virCPUarmVendorFindByID(virCPUarmMapPtr map,
> +unsigned long vendor_id)
> +{
> +size_t i;
> +
> +for (i = 0; i < map->nvendors; i++) {
> +if (map->vendors[i]->value == vendor_id)
> +return map->vendors[i];
> +}
> +
> +return NULL;
> +}
> +
> +
> +static virCPUarmVendorPtr
> +virCPUarmVendorFindByName(virCPUarmMapPtr map,
> + const char *name)
> +{
> +size_t i;
> +
> +for (i = 0; i < map->nvendors; i++) {
> +if (STREQ(map->vendors[i]->name, name))
> +return map->vendors[i];
> +}
> +
> +return NULL;
> +}
> +
> +
> +static int
> +virCPUarmVendorParse(xmlXPathContextPtr ctxt,
> + const char *name,
> + void *data)
The indentation is still wrong.
> +{
> +virCPUarmMapPtr map = data;
> +g_autoptr(virCPUarmVendor) vendor = NULL;
> +
> +vendor = g_new0(virCPUarmVendor, 1);
> +vendor->name = g_strdup(name);
> +
> +if (virCPUarmVendorFindByName(map, vendor->name)) {
> +virReportError(VIR_ERR_INTERNAL_ERROR,
> + _("CPU vendor %s already defined"),
> + vendor->name);
> +return -1;
> +}
> +
> +if (virXPathULongHex("string(@value)", ctxt, >value) < 0) {
> +virReportError(VIR_ERR_INTERNAL_ERROR,
> + "%s", _("Missing CPU vendor value"));
> +return -1;
> +}
> +
> +if (virCPUarmVendorFindByID(map, vendor->value)) {
> +virReportError(VIR_ERR_INTERNAL_ERROR,
> + _("CPU vendor value 0x%2lx already defined"),
> + vendor->value);
> +return -1;
> +}
> +
> +if (VIR_APPEND_ELEMENT(map->vendors, map->nvendors, vendor) < 0)
> +return -1;
> +
> +return 0;
> +}
> +
> +static virCPUarmModelPtr
> +virCPUarmModelFind(virCPUarmMapPtr map,
> + const char *name)
> +{
> +size_t i;
> +
> +for (i = 0; i < map->nmodels; i++) {
> +if (STREQ(map->models[i]->name, name))
> +return map->models[i];
> +}
> +
> +return NULL;
> +}
> +
> +#if defined(__aarch64__)
> +static virCPUarmModelPtr
> +virCPUarmModelFindByPVR(virCPUarmMapPtr map,
> +unsigned long pvr)
> +{
> +size_t i;
> +
> +for (i = 0; i < map->nmodels; i++) {
> +if (map->models[i]->data.pvr == pvr)
> +return map->models[i];
> +}
> +
> +return NULL;
> +}
> +#endif
Apparently I didn't notice this in my last review...
virCPUarmModelFindByPVR should be moved to the next patch, otherwise
compilation would fail after this patch as the function is never used
here.
> +
> +static int
> +virCPUarmModelParse(xmlXPathContextPtr ctxt,
> + const char *name,
> + void *data)
Wrong indentation again.
> +{
> +virCPUarmMapPtr map = data;
> +virCPUarmModel *model;
g_autoptr(virCPUarmModel) model = NULL;
> +g_autofree xmlNodePtr *nodes = NULL;
> +g_autofree char *vendor = NULL;
> +
> +model = g_new0(virCPUarmModel, 1);
> +model->name = g_strdup(name);
> +
> +if (virCPUarmModelFind(map, model->name)) {
> +virReportError(VIR_ERR_INTERNAL_ERROR,
> + _("CPU model %s already defined"),
> + model->name);
> +return -1;
> +}
> +
> +if (virXPathBoolean("boolean(./vendor)", ctxt)) {
> +vendor = virXPathString("string(./vendor/@name)", ctxt);
> +if (!vendor) {
> +virReportError(VIR_ERR_INTERNAL_ERROR,
> + _("Invalid vendor element in CPU model %s"),
> + model->name);
> +return -1;
> +}
> +
> +if (!(model->vendor = virCPUarmVendorFindByName(map, vendor))) {
> +virReportError(VIR_ERR_INTERNAL_ERROR,
> + _("Unknown vendor %s referenced by CPU model %s"),
> + vendor, model->name);
> +return -1;
> +}
> +}
> +
> +if (!virXPathBoolean("boolean(./pvr)", ctxt)) {
> +
