Re: [PATCH] udevProcessCSS: fix segfault
On Mon, Sep 21, 2020 at 07:06:32PM +0200, Marc Hartmayer wrote:
> Don't process subchannel devices where `def->driver` is not set. This
> fixes the following segfault:
>
> Thread 21 "nodedev-init" received signal SIGSEGV, Segmentation fault.
> [Switching to Thread 0x3ffb08fc910 (LWP 64303)]
> (gdb) bt
> #0 0x03fffd1272b4 in __strcmp_vx () at /lib64/libc.so.6
> #1 0x03ffc260c3a8 in udevProcessCSS (device=0x3ff9018d130,
> def=0x3ff90194a90)
> #2 0x03ffc260cb78 in udevGetDeviceDetails (device=0x3ff9018d130,
> def=0x3ff90194a90)
> #3 0x03ffc260d126 in udevAddOneDevice (device=0x3ff9018d130)
> #4 0x03ffc260d414 in udevProcessDeviceListEntry (udev=0x3ffa810d800,
> list_entry=0x3ff90001990)
> #5 0x03ffc260d638 in udevEnumerateDevices (udev=0x3ffa810d800)
> #6 0x03ffc260e08e in nodeStateInitializeEnumerate (opaque=0x3ffa810d800)
> #7 0x03fffdaa14b6 in virThreadHelper (data=0x3ffa810df00)
> #8 0x03fffc309ed6 in start_thread ()
> #9 0x03fffd185e66 in thread_start ()
> (gdb) p *def
> $2 = {
> name = 0x0,
> sysfs_path = 0x3ff90198e80 "/sys/devices/css0/0.0.ff40",
Okay, this patch fixes the segfault. However, if ^this generated it because the
driver name is not set, how do we even get to the resulting device tree as
outlined in 05e6cdafa6e0?
+- css_0_0_003a
|
+- ccw_0_0_1a2b
|
+- scsi_host0
What kind of CSS device is the one causing the error? If we skip this CSS
device, we don't generate a name for it and don't put it in the list, so I'm
quite puzzled on what I missed in the IBM document and thus in the review
process.
FWIW:
Reviewed-by: Erik Skultety
[PATCH] libvirt: ensure defresult is used in virConnectAuthCallbackDefault
A previous change to this function's password handling broke the use of default values for credential types other than VIR_CRED_PASSPHRASE and VIR_CRED_NOECHOPROMPT. Signed-off-by: Matt Coleman --- src/libvirt.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/src/libvirt.c b/src/libvirt.c index 0748eb2352..63c8bdea9f 100644 --- a/src/libvirt.c +++ b/src/libvirt.c @@ -146,7 +146,9 @@ virConnectAuthCallbackDefault(virConnectCredentialPtr cred, len = strlen(buf); if (len != 0 && buf[len-1] == '\n') buf[len-1] = '\0'; -bufptr = g_strdup(buf); + +if (strlen(buf) > 0) +bufptr = g_strdup(buf); break; case VIR_CRED_PASSPHRASE: -- 2.27.0
Re: [PATCH] libvirt: ensure defresult is used in virConnectAuthCallbackDefault
On Mon, Sep 21, 2020 at 10:02 PM Matt Coleman wrote: > > A previous change to this function's password handling broke the use of > default values for credential types other than VIR_CRED_PASSPHRASE and > VIR_CRED_NOECHOPROMPT. > > Signed-off-by: Matt Coleman > --- > src/libvirt.c | 4 +++- > 1 file changed, 3 insertions(+), 1 deletion(-) > > diff --git a/src/libvirt.c b/src/libvirt.c > index 0748eb2352..63c8bdea9f 100644 > --- a/src/libvirt.c > +++ b/src/libvirt.c > @@ -146,7 +146,9 @@ virConnectAuthCallbackDefault(virConnectCredentialPtr > cred, > len = strlen(buf); > if (len != 0 && buf[len-1] == '\n') > buf[len-1] = '\0'; > -bufptr = g_strdup(buf); > + > +if (strlen(buf) > 0) > +bufptr = g_strdup(buf); > break; > > case VIR_CRED_PASSPHRASE: > -- > 2.27.0 > > Reviewed-by: Neal Gompa -- 真実はいつも一つ！/ Always, there's only one truth!
Re: [PATCH] util: support PCI passthrough net device stats collection
(Please don't Cc individual developers in patch submissions unless
they've specifically asked you to do so)
On 9/21/20 8:25 AM, zhenwei pi wrote:
Collect PCI passthrough net device stats from kernel by netlink
API.
Currently, libvirt can not get PCI passthrough net device stats,
run command:
#virsh domifstat instance --interface=52:54:00:2d:b2:35
error: Failed to get interface stats instance 52:54:00:2d:b2:35
error: internal error: Interface name not provided
The PCI device(usually SR-IOV virtual function device) is detached
while it's used in PCI passthrough mode. And we can not parse this
device from /proc/net/dev any more.
In this patch, libvirt check net device is VF of not firstly, then
query virNetDevVFInterfaceStats(new API).
virNetDevVFInterfaceStats parses VFs info of all PFs, compares MAC
address until the two MAC addresses match.
'#ip -s link show' can get the same result. Instead of parsing the
output result, implement this feature by libnl API.
Notice that this feature deponds on driver of PF.
Test on Mellanox ConnectX-4 Lx, it works well.
Also test on Intel Corporation 82599ES, it works, but only get 0.
(ip-link command get the same result).
Signed-off-by: zhenwei pi
---
meson.build | 4 ++
src/libvirt_private.syms | 1 +
src/qemu/qemu_driver.c | 3 +
src/util/virnetdev.c | 158 +++
src/util/virnetdev.h | 5 ++
5 files changed, 171 insertions(+)
diff --git a/meson.build b/meson.build
index 24535a403c..e17da9e2b9 100644
--- a/meson.build
+++ b/meson.build
@@ -1392,6 +1392,10 @@ if not get_option('virtualport').disabled()
endif
endif
+if cc.has_header_symbol('linux/if_link.h', 'IFLA_VF_STATS')
+ conf.set('WITH_VF_STATS', 1)
+endif
+
(a bit of digression here...)
I understand why you put this chunk in, but I'm fairly certain that it
isn't needed.
In the case of the few other things from if_link.h that have their own
"WITH_BLAH" compile-time flag (e.g. WITH_VIRTUALPORT, which checks for
existence of IFLA_PORT_MAX in if_link.h), they were added in the before
before time, when the feature in question had been recently added to the
kernel, and so there were some supported distro versions that had the
new feature, and some that didn't yet. In order to compile properly on
all supported platforms (though lacking these new-at-the-time features
on some) we had to gate them on the presence of some sentinel in
if_link.h. Those WITH_BLAH flags were then forgotten about, even though
the list of supported platform/versions has changed over the years, and
they will now work on *any* supported Linux platform.
Now that this patch has pointed them out, I think it's time for some
house cleaning. IFLA_PORT_MAX, for example, has been in the upstream
kernel since 2.6.35, and was even backported to the RHEL*6* 2.6.32
kernel. Even router firmware these days has a newer kernel than that, so
it's a safe bet that any system with __linux_ and WITH_LIBNL can enable
all of that code, i.e. we can just get rid of the extra
WITH_VIRTUALPORT. I just added this to my personal todo list; let's see
how long it takes until I actually accomplish it (betting starts now!)
Back to the topic of *this* patch: I looked up IFLA_VF_STATS for the
upstream kernel, and it first appeared in kernel 4.2. RHEL7 (which is
the oldest RHEL now supported by libvirt) is based on kernel 3.10, but
has *a lot* of backports, and so when I checked I found that it also
supports IFLA_VF_STATS. I don't have the details handy for the oldest
version of other Linuxes we support, but I tried eliminating the extra
check for IFLA_VF_STATS, and the full CI suite on gitlab builds without
error (there is an unrelated error in ubuntu-18.04 in libxl, and
different errors in the freebsd, mingw, and macos builds that *are*
caused by different aspects of this patch, but IFLA_VF_STATS does exist
on all Linux platforms included in the libvirt upstream CI testing).
So support for this feature can be gated on "defined(__linux__) &&
WITH_LIBNL (as should just about anything else using netlink)
if host_machine.system() == 'windows'
ole32_dep = cc.find_library('ole32')
oleaut32_dep = cc.find_library('oleaut32')
diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
index bdbe3431b8..bcc40b8d69 100644
--- a/src/libvirt_private.syms
+++ b/src/libvirt_private.syms
@@ -2585,6 +2585,7 @@ virNetDevSetRcvMulti;
virNetDevSetupControl;
virNetDevSysfsFile;
virNetDevValidateConfig;
+virNetDevVFInterfaceStats;
# util/virnetdevbandwidth.h
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index ae715c01d7..f554010c40 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -10196,6 +10196,9 @@ qemuDomainInterfaceStats(virDomainPtr dom,
if (virDomainNetGetActualType(net) == VIR_DOMAIN_NET_TYPE_VHOSTUSER) {
if (virNetDevOpenvswitchInterfaceStats(net->ifname, stats) < 0
Re: [PATCH V3] Modify virCPUarmCompare to perform compare actions
Ping for reviews.
Thanks in advance
On Wed, Sep 16, 2020 at 4:59 PM Zhenyu Zheng
wrote:
> Modify virCPUarmCompare in cpu_arm.c to perform compare action.
> This patch only adds host to host CPU compare, the rest cases
> remains the same. This is useful for source and destination host
> compare during migrations to avoid migration between different
> CPU models that have different CPU freatures.
>
> Signed-off-by: Zhenyu Zheng
> ---
> src/cpu/cpu_arm.c | 43 +++
> 1 file changed, 39 insertions(+), 4 deletions(-)
>
> diff --git a/src/cpu/cpu_arm.c b/src/cpu/cpu_arm.c
> index 939a3b8390..b420b14e86 100644
> --- a/src/cpu/cpu_arm.c
> +++ b/src/cpu/cpu_arm.c
> @@ -463,11 +463,46 @@ virCPUarmBaseline(virCPUDefPtr *cpus,
> }
>
> static virCPUCompareResult
> -virCPUarmCompare(virCPUDefPtr host G_GNUC_UNUSED,
> - virCPUDefPtr cpu G_GNUC_UNUSED,
> - bool failMessages G_GNUC_UNUSED)
> +virCPUarmCompare(virCPUDefPtr host,
> + virCPUDefPtr cpu,
> + bool failIncompatible
> +)
> {
> -return VIR_CPU_COMPARE_IDENTICAL;
> +virCPUCompareResult ret = VIR_CPU_COMPARE_IDENTICAL;
> +
> +/* Only support host to host CPU compare for ARM*/
> +if (cpu->type != VIR_CPU_TYPE_HOST)
> +return ret;
> +
> +if (!host || !host->model) {
> +if (failIncompatible) {
> +virReportError(VIR_ERR_CPU_INCOMPATIBLE, "%s",
> + _("unknown host CPU"));
> +ret = VIR_CPU_COMPARE_ERROR;
> +} else {
> +VIR_WARN("unknown host CPU");
> +ret = VIR_CPU_COMPARE_INCOMPATIBLE;
> +}
> +return ret;
> +}
> +
> +/* Compare vendor and model to check if CPUs are identical */
> +if (STRNEQ(host->vendor, cpu->vendor) ||
> +STRNEQ(host->model, cpu->model)) {
> +VIR_DEBUG("Host CPU model does not match required CPU model %s",
> + cpu->model);
> +
> +if (failIncompatible) {
> +ret = VIR_CPU_COMPARE_ERROR;
> +virReportError(VIR_ERR_CPU_INCOMPATIBLE,
> + _("Host CPU model does not match required CPU
> model %s"),
> + cpu->model);
> +} else {
> +ret = VIR_CPU_COMPARE_INCOMPATIBLE;
> +}
> +}
> +
> +return ret;
> }
>
> static int
> --
> 2.20.1
>
>
>
Re: [PATCH 0/2] qemu: Use .hostdevice attribute for usb-host
On 9/14/20 4:57 AM, Michal Privoznik wrote: See 2/2 for explanation why we need this. Also, .hostdevice uses qemu_open() under the hood which enables Libvirt to pass FD instead of /dev/bus/usb/... path. However, qemu_open() does not support getfd style of passing FD (which is what we use), but so called FD sets which require some more work. I'm working on it as we speak but I figured, let's fix this bug first and post FD passing after that. For curious ones, here is the raw, unclean version with FD sets: https://gitlab.com/MichalPrivoznik/libvirt/-/commits/qemu_hostdev_alt/ All patches: Reviewed-by: Daniel Henrique Barboza Michal Prívozník (2): qemu_capabilities: Add QEMU_CAPS_USB_HOST_HOSTDEVICE qemu: Use .hostdevice attribute for usb-host src/qemu/qemu_capabilities.c | 10 ++ src/qemu/qemu_capabilities.h | 3 + src/qemu/qemu_command.c | 21 ++- .../caps_1.5.3.x86_64.replies | 126 -- .../caps_1.6.0.x86_64.replies | 126 -- .../caps_1.7.0.x86_64.replies | 126 -- .../caps_2.1.1.x86_64.replies | 126 -- .../caps_2.10.0.aarch64.replies | 134 +-- .../caps_2.10.0.ppc64.replies | 130 -- .../caps_2.10.0.s390x.replies | 134 +-- .../caps_2.10.0.x86_64.replies| 146 +--- .../caps_2.11.0.s390x.replies | 134 +-- .../caps_2.11.0.x86_64.replies| 146 +--- .../caps_2.12.0.aarch64.replies | 150 +--- .../caps_2.12.0.ppc64.replies | 142 --- .../caps_2.12.0.s390x.replies | 142 --- .../caps_2.12.0.x86_64.replies| 162 ++ .../caps_2.4.0.x86_64.replies | 126 -- .../caps_2.5.0.x86_64.replies | 130 -- .../caps_2.6.0.aarch64.replies| 134 +-- .../caps_2.6.0.ppc64.replies | 130 -- .../caps_2.6.0.x86_64.replies | 130 -- .../caps_2.7.0.s390x.replies | 130 -- .../caps_2.7.0.x86_64.replies | 130 -- .../caps_2.8.0.s390x.replies | 134 +-- .../caps_2.8.0.x86_64.replies | 130 -- .../caps_2.9.0.ppc64.replies | 130 -- .../caps_2.9.0.s390x.replies | 134 +-- .../caps_2.9.0.x86_64.replies | 146 +--- .../caps_3.0.0.ppc64.replies | 142 --- .../caps_3.0.0.riscv32.replies| 138 +-- .../caps_3.0.0.riscv64.replies| 138 +-- .../caps_3.0.0.s390x.replies | 142 --- .../caps_3.0.0.x86_64.replies | 162 ++ .../caps_3.1.0.ppc64.replies | 142 --- .../caps_3.1.0.x86_64.replies | 162 ++ .../caps_4.0.0.aarch64.replies| 150 +--- .../caps_4.0.0.ppc64.replies | 142 --- .../caps_4.0.0.riscv32.replies| 138 +-- .../caps_4.0.0.riscv64.replies| 138 +-- .../caps_4.0.0.s390x.replies | 142 --- .../caps_4.0.0.x86_64.replies | 162 ++ .../caps_4.1.0.x86_64.replies | 154 ++--- .../caps_4.2.0.aarch64.replies| 154 ++--- .../caps_4.2.0.ppc64.replies | 142 --- .../caps_4.2.0.s390x.replies | 142 --- .../caps_4.2.0.x86_64.replies | 154 ++--- .../caps_5.0.0.aarch64.replies| 154 ++--- .../caps_5.0.0.ppc64.replies | 142 --- .../caps_5.0.0.riscv64.replies| 138 +-- .../caps_5.0.0.x86_64.replies | 154 ++--- .../caps_5.1.0.x86_64.replies | 158 ++--- .../caps_5.1.0.x86_64.xml | 1 + .../caps_5.2.0.x86_64.replies | 158 ++--- .../caps_5.2.0.x86_64.xml | 1 + 55 files changed, 6110 insertions(+), 982 deletions(-)
Re: [RFC] Add basic driver for the Cloud-Hypervisor
On Tue, Sep 15, 2020 at 5:53 AM Daniel P. Berrangé wrote:
>
> On Thu, Aug 27, 2020 at 11:24:32AM -0700, William Douglas wrote:
> > +virCHMonitorPtr
> > +virCHMonitorNew(virDomainObjPtr vm, const char *socketdir)
> > +{
> > +virCHMonitorPtr ret = NULL;
> > +virCHMonitorPtr mon = NULL;
> > +virCommandPtr cmd = NULL;
> > +int pings = 0;
> > +
> > +if (virCHMonitorInitialize() < 0)
> > +return NULL;
> > +
> > +if (!(mon = virObjectLockableNew(virCHMonitorClass)))
> > +return NULL;
> > +
> > +mon->socketpath = g_strdup_printf("%s/%s-socket", socketdir,
> > vm->def->name);
> > +
> > +/* prepare to launch Cloud-Hypervisor socket */
> > +if (!(cmd = chMonitorBuildSocketCmd(vm, mon->socketpath)))
> > +goto cleanup;
> > +
> > +if (virFileMakePath(socketdir) < 0) {
> > +virReportSystemError(errno,
> > + _("Cannot create socket directory '%s'"),
> > + socketdir);
> > +goto cleanup;
> > +}
> > +
> > +/* launch Cloud-Hypervisor socket */
> > +if (virCommandRunAsync(cmd, &mon->pid) < 0)
> > +goto cleanup;
> > +
> > +/* get a curl handle */
> > +mon->handle = curl_easy_init();
> > +
> > +/* try to ping VMM socket 5 times to make sure it is ready */
> > +while (pings < 5) {
> > +if (virCHMonitorPingVMM(mon) == 0)
> > +break;
> > +if (pings == 5)
> > +goto cleanup;
> > +
> > +g_usleep(100 * 1000);
> > +}
>
> This is highly undesirable. Is there no way to launch the CH process
> such that the socket is guaranteed to be accepting requests by the
> time it has forked into the background ? Or is there a way to pass
> in a pre-opened FD for the monitor socket ?
>
> This kind of wait with timeout will cause startup failures due to
> timeout under load.
Currently the cloud-hypervisor process doesn't fork into the
background so that was initially what I was thinking to add to
cloud-hypervsior. Would tracking the pid of the cloud-hypervsior
running in the background be required at that point (assuming the
socket path to control the daemon would be known and working given
virCommandRun returns successfully)?
> What is the security model for the processes ? The libvirt driver
> here is running as root and spawning CH as root. We don't really
> want the VM process running as root though. It really needs to
> be unprivileged from a DAC POV. Obviously that's quite a bit more
> work for you todo, but it should be opssible to share alot of the
> security mgmt infrastructure that we already have for QEMU and
> LXC drivers. It can manage DAC permissions and apply SELinux or
> AppArmor MAC labelling/profiles.
Currently cloud-hypervisor runs with cap_net_admin permitted and
effective on it. The implementation will be updated to run under the
user's session using a non-root driver (with its own daemon as you had
mentioned earlier).
>
> The other big question is around device addressing. If using PCI,
> then PCI address assignment logic is critical to ensure consistent
> guest ABI.
>
I'll be adding the device passthrough for the actual driver submission
but if you would like to see an updated RFC with that added I could do
so.
> 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 :|
>
Thank you very much for the very thorough comments, I'll be updating
my patch addressing all your feedback.
William
Re: [PATCH] tests: avoid close of bad file handle in commandtest
On 9/21/20 12:38 PM, Daniel P. Berrangé wrote:
Closed file handles need to be initialized to -1, not 0. This caused a
inappropriate double close of stdin, which is not desirable, although
it had no ill effects.
Signed-off-by: Daniel P. Berrangé
---
tests/commandtest.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
Reviewed-by: Eric Blake
diff --git a/tests/commandtest.c b/tests/commandtest.c
index 42225a8ef2..cbbcda4e5f 100644
--- a/tests/commandtest.c
+++ b/tests/commandtest.c
@@ -1091,8 +1091,8 @@ static int test27(const void *unused G_GNUC_UNUSED)
printf("Could not set send buffers\n");
goto cleanup;
}
-pipe1[1] = 0;
-pipe2[1] = 0;
+pipe1[1] = -1;
+pipe2[1] = -1;
buffer1 = NULL;
buffer2 = NULL;
--
Eric Blake, Principal Software Engineer
Red Hat, Inc. +1-919-301-3226
Virtualization: qemu.org | libvirt.org
Re: [libvirt PATCH] tests: don't mix FILE* and UNIX FD I/O on same stream
On 9/21/20 12:36 PM, Daniel P. Berrangé wrote: There is currently a hand in test27 that exhibits itself on FreeBSD 11.4 s/hand/hang/ only. The behaviour is that virCommandProcessIO gets POLLIN on the FD for stdout, but read() blocks. Meanwhile commandtest also blocks in write for stderr because the pipe buffers are full. This fix in commandhelper likely does not really address the root cause just hides it due to the buffering done by FILE *. Mixing UNIX FD I/O and FILE * I/O is bad practice regardles. regardless POSIX has rules for when it is safe (it has a notion of an active handle, and what must be done to a FILE* that is currently the active handle before doing further I/O via an fd that wants to become the active handle https://pubs.opengroup.org/onlinepubs/9699919799/functions/V2_chap02.html#tag_15_05). But you're right that not mixing is the easiest approach. Signed-off-by: Daniel P. Berrangé --- tests/commandhelper.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) With typos fixed, Reviewed-by: Eric Blake -- Eric Blake, Principal Software Engineer Red Hat, Inc. +1-919-301-3226 Virtualization: qemu.org | libvirt.org
[PATCH] tests: avoid close of bad file handle in commandtest
Closed file handles need to be initialized to -1, not 0. This caused a
inappropriate double close of stdin, which is not desirable, although
it had no ill effects.
Signed-off-by: Daniel P. Berrangé
---
tests/commandtest.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/tests/commandtest.c b/tests/commandtest.c
index 42225a8ef2..cbbcda4e5f 100644
--- a/tests/commandtest.c
+++ b/tests/commandtest.c
@@ -1091,8 +1091,8 @@ static int test27(const void *unused G_GNUC_UNUSED)
printf("Could not set send buffers\n");
goto cleanup;
}
-pipe1[1] = 0;
-pipe2[1] = 0;
+pipe1[1] = -1;
+pipe2[1] = -1;
buffer1 = NULL;
buffer2 = NULL;
--
2.26.2
[libvirt PATCH] tests: don't mix FILE* and UNIX FD I/O on same stream
There is currently a hand in test27 that exhibits itself on FreeBSD 11.4
only. The behaviour is that virCommandProcessIO gets POLLIN on the
FD for stdout, but read() blocks. Meanwhile commandtest also blocks
in write for stderr because the pipe buffers are full.
This fix in commandhelper likely does not really address the root cause
just hides it due to the buffering done by FILE *. Mixing UNIX FD I/O
and FILE * I/O is bad practice regardles.
Signed-off-by: Daniel P. Berrangé
---
tests/commandhelper.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/tests/commandhelper.c b/tests/commandhelper.c
index 05f577730f..7c260c4e13 100644
--- a/tests/commandhelper.c
+++ b/tests/commandhelper.c
@@ -221,9 +221,9 @@ int main(int argc, char **argv) {
}
for (i = 0; i < numpollfds; i++) {
-if (write(STDOUT_FILENO, buffers[i], buflen[i]) != buflen[i])
+if (fwrite(buffers[i], 1, buflen[i], stdout) != buflen[i])
goto cleanup;
-if (write(STDERR_FILENO, buffers[i], buflen[i]) != buflen[i])
+if (fwrite(buffers[i], 1, buflen[i], stderr) != buflen[i])
goto cleanup;
}
--
2.26.2
[PATCH] udevProcessCSS: fix segfault
Don't process subchannel devices where `def->driver` is not set. This
fixes the following segfault:
Thread 21 "nodedev-init" received signal SIGSEGV, Segmentation fault.
[Switching to Thread 0x3ffb08fc910 (LWP 64303)]
(gdb) bt
#0 0x03fffd1272b4 in __strcmp_vx () at /lib64/libc.so.6
#1 0x03ffc260c3a8 in udevProcessCSS (device=0x3ff9018d130,
def=0x3ff90194a90)
#2 0x03ffc260cb78 in udevGetDeviceDetails (device=0x3ff9018d130,
def=0x3ff90194a90)
#3 0x03ffc260d126 in udevAddOneDevice (device=0x3ff9018d130)
#4 0x03ffc260d414 in udevProcessDeviceListEntry (udev=0x3ffa810d800,
list_entry=0x3ff90001990)
#5 0x03ffc260d638 in udevEnumerateDevices (udev=0x3ffa810d800)
#6 0x03ffc260e08e in nodeStateInitializeEnumerate (opaque=0x3ffa810d800)
#7 0x03fffdaa14b6 in virThreadHelper (data=0x3ffa810df00)
#8 0x03fffc309ed6 in start_thread ()
#9 0x03fffd185e66 in thread_start ()
(gdb) p *def
$2 = {
name = 0x0,
sysfs_path = 0x3ff90198e80 "/sys/devices/css0/0.0.ff40",
parent = 0x0,
parent_sysfs_path = 0x0,
parent_wwnn = 0x0,
parent_wwpn = 0x0,
parent_fabric_wwn = 0x0,
driver = 0x0,
devnode = 0x0,
devlinks = 0x3ff90194670,
caps = 0x3ff90194380
}
Fixes: 05e6cdafa6e0 ("node_device: detect CSS devices")
Reviewed-by: Boris Fiuczynski
Signed-off-by: Marc Hartmayer
---
src/node_device/node_device_udev.c | 5 +++--
1 file changed, 3 insertions(+), 2 deletions(-)
diff --git a/src/node_device/node_device_udev.c
b/src/node_device/node_device_udev.c
index 5f2841bb7d8e..12e3f30badd1 100644
--- a/src/node_device/node_device_udev.c
+++ b/src/node_device/node_device_udev.c
@@ -1130,8 +1130,9 @@ udevProcessCSS(struct udev_device *device,
virNodeDeviceDefPtr def)
{
/* only process IO subchannel and vfio-ccw devices to keep the list sane */
-if (STRNEQ(def->driver, "io_subchannel") &&
-STRNEQ(def->driver, "vfio_ccw"))
+if (!def->driver ||
+(STRNEQ(def->driver, "io_subchannel") &&
+ STRNEQ(def->driver, "vfio_ccw")))
return -1;
if (udevGetCCWAddress(def->sysfs_path, &def->caps->data) < 0)
--
2.25.4
Re: [PATCH v5 0/8] Configurable policy for handling deprecated interfaces
On Mon, 21 Sep 2020 at 15:58, Markus Armbruster wrote:
>
> Peter Maydell writes:
> > If this only covers QMP, should we make the argument to -compat
> > have a name that expresses that? eg deprecated-qmp-input,
> > deprecated-qmp-output ?
>
> It's only implemented for QMP so far. But we really want it for all
> external interfaces for use by machines. Today, that's QMP and CLI.
>
> Naming the parameters deprecated-qmp-{input,output} leads to separate
> settings for QMP and CLI.
I think that's a good thing. I might have fixed up my handling
of QMP to avoid deprecated behaviours but not yet got round to
doing that for my command line option choices (or vice-versa).
> > Also, it seems a bit repetitive to say 'deprecated' here all
> > the time -- do you have a future use of -compat in mind which
> > would be to adjust something that is *not* deprecated ? If
> > not, maybe the 'deprecated' part should be in the option name
> > rather than in every argument, eg
> >
> > -deprecation-compat qmp-input=crash,qmp-output=hide,cli-option=reject
> >
> > ?
>
> My cover letter hints at such future uses: "We may want to extend it to
> cover [...] experimental features."
Ah, I read "-compat covers only deprecated syntactic aspects of QMP. We
may want to extend it to cover semantic aspects, CLI, and experimental
features." as implying "deprecated semantic aspects, deprecated CLI,
and deprecated experimental features"...
thanks
-- PMM
Re: [PATCH v5 0/8] Configurable policy for handling deprecated interfaces
Peter Maydell writes:
> On Mon, 14 Sep 2020 at 09:55, Markus Armbruster wrote:
>>
>> New option -compat lets you configure what to do when deprecated
>> interfaces get used. This is intended for testing users of the
>> management interfaces. It is experimental.
>>
>> -compat deprecated-input= configures what to do when
>> deprecated input is received. Available policies:
>>
>> * accept: Accept deprecated commands and arguments (default)
>> * reject: Reject them
>> * crash: Crash
>>
>> -compat deprecated-output= configures what to do when
>> deprecated output is sent. Available output policies:
>>
>> * accept: Emit deprecated command results and events (default)
>> * hide: Suppress them
>>
>> For now, -compat covers only deprecated syntactic aspects of QMP. We
>> may want to extend it to cover semantic aspects, CLI, and experimental
>> features.
>
> Some bikeshedding on option naming...
>
> If this only covers QMP, should we make the argument to -compat
> have a name that expresses that? eg deprecated-qmp-input,
> deprecated-qmp-output ?
It's only implemented for QMP so far. But we really want it for all
external interfaces for use by machines. Today, that's QMP and CLI.
Naming the parameters deprecated-qmp-{input,output} leads to separate
settings for QMP and CLI.
Naming them just deprecated-{input,output} leads to a single set of
settings common to all externeal interfaces, or to sugar for setting all
the deprecated-*-{input,output} we may have.
I don't think getting it wrong now would be a big deal. No excuse for
getting it wrong unthinkingly :)
> Also, it seems a bit repetitive to say 'deprecated' here all
> the time -- do you have a future use of -compat in mind which
> would be to adjust something that is *not* deprecated ? If
> not, maybe the 'deprecated' part should be in the option name
> rather than in every argument, eg
>
> -deprecation-compat qmp-input=crash,qmp-output=hide,cli-option=reject
>
> ?
My cover letter hints at such future uses: "We may want to extend it to
cover [...] experimental features." Something like
-compat experimental-input=reject,experimental-output=hide
Re: Plans for the next release
On Mon, Sep 21, 2020 at 14:38:55 +0200, Ján Tomko wrote: > [please don't top-post on technical mailing lists] > > On a Monday in 2020, Alexander Wels wrote: > >Which version would that be? 6.7.0? > > > > No, 6.7.0 is already out: > https://www.redhat.com/archives/libvirt-announce/2020-September/msg0.html > > The next release is 6.8.0 > > Since 2.0.0 we follow a time-based versioning scheme: > https://libvirt.org/downloads.html#numbering And if you are not sure, you can always check meson.build, which contains the upcoming release (it is incremented immediately after each release). Jirka
[libvirt PATCH] virgdbus: add DBus reply format check
We used to check the format of reply data with libdbus so we should do
the same with GLib DBus as well.
Signed-off-by: Pavel Hrdina
---
At first I thought that this is not necessary as it is unlikely to
happen but after Jano found the bug with firewalld getZones function
and asked about checking return values I figured out that it will be
better to check it because if the returned message would have different
format it would be silently ignored.
src/rpc/virnetdaemon.c | 1 +
src/util/virfirewalld.c | 5 +
src/util/virgdbus.c | 8 ++--
src/util/virgdbus.h | 2 ++
src/util/virpolkit.c| 1 +
src/util/virsystemd.c | 7 +++
6 files changed, 22 insertions(+), 2 deletions(-)
diff --git a/src/rpc/virnetdaemon.c b/src/rpc/virnetdaemon.c
index 12d4d9bf87..f3a5e9f75c 100644
--- a/src/rpc/virnetdaemon.c
+++ b/src/rpc/virnetdaemon.c
@@ -487,6 +487,7 @@ virNetDaemonCallInhibit(virNetDaemonPtr dmn,
rc = virGDBusCallMethodWithFD(systemBus,
&reply,
+ G_VARIANT_TYPE("(h)"),
&replyFD,
NULL,
"org.freedesktop.login1",
diff --git a/src/util/virfirewalld.c b/src/util/virfirewalld.c
index 12448f0681..a94ac7c183 100644
--- a/src/util/virfirewalld.c
+++ b/src/util/virfirewalld.c
@@ -95,6 +95,7 @@ virFirewallDGetVersion(unsigned long *version)
if (virGDBusCallMethod(sysbus,
&reply,
+ G_VARIANT_TYPE("(v)"),
NULL,
VIR_FIREWALL_FIREWALLD_SERVICE,
"/org/fedoraproject/FirewallD1",
@@ -147,6 +148,7 @@ virFirewallDGetBackend(void)
if (virGDBusCallMethod(sysbus,
&reply,
+ G_VARIANT_TYPE("(v)"),
error,
VIR_FIREWALL_FIREWALLD_SERVICE,
"/org/fedoraproject/FirewallD1/config",
@@ -207,6 +209,7 @@ virFirewallDGetZones(char ***zones, size_t *nzones)
if (virGDBusCallMethod(sysbus,
&reply,
+ G_VARIANT_TYPE("(as)"),
NULL,
VIR_FIREWALL_FIREWALLD_SERVICE,
"/org/fedoraproject/FirewallD1",
@@ -295,6 +298,7 @@ virFirewallDApplyRule(virFirewallLayer layer,
if (virGDBusCallMethod(sysbus,
&reply,
+ G_VARIANT_TYPE("(s)"),
error,
VIR_FIREWALL_FIREWALLD_SERVICE,
"/org/fedoraproject/FirewallD1",
@@ -357,6 +361,7 @@ virFirewallDInterfaceSetZone(const char *iface,
message = g_variant_new("(ss)", zone, iface);
return virGDBusCallMethod(sysbus,
+ NULL,
NULL,
NULL,
VIR_FIREWALL_FIREWALLD_SERVICE,
diff --git a/src/util/virgdbus.c b/src/util/virgdbus.c
index 535b19f0a4..837c8faf1f 100644
--- a/src/util/virgdbus.c
+++ b/src/util/virgdbus.c
@@ -181,6 +181,7 @@ virGDBusCloseSystemBus(void)
* virGDBusCallMethod:
* @conn: a DBus connection
* @reply: pointer to receive reply message, or NULL
+ * @replyType: pointer to GVariantType to validate reply data, or NULL
* @error: libvirt error pointer or NULL
* @busName: bus identifier of the target service
* @objectPath: object path of the target service
@@ -198,6 +199,7 @@ virGDBusCloseSystemBus(void)
int
virGDBusCallMethod(GDBusConnection *conn,
GVariant **reply,
+ const GVariantType *replyType,
virErrorPtr error,
const char *busName,
const char *objectPath,
@@ -220,7 +222,7 @@ virGDBusCallMethod(GDBusConnection *conn,
ifaceName,
method,
data,
- NULL,
+ replyType,
G_DBUS_CALL_FLAGS_NONE,
VIR_DBUS_METHOD_CALL_TIMEOUT_MILIS,
NULL,
@@ -250,6 +252,7 @@ virGDBusCallMethod(GDBusConnection *conn,
int
virGDBusCallMethodWithFD(GDBusConnection *conn,
GVariant **reply,
+ const GVariantType *replyType,
GUnixFDList **replyFD,
virErrorPtr error,
const char *busName,
@@ -274,7 +277,7 @@ virGDBusCallMethodWithFD(GDBusConnection *conn,
ifaceName,
method,
[libvirt PATCH 6/9] cpu: Validate XML
Signed-off-by: Tim Wiederhake
---
src/bhyve/bhyve_driver.c | 2 +-
src/conf/cpu_conf.c | 28
src/conf/cpu_conf.h | 6 --
src/conf/domain_conf.c | 3 ++-
src/cpu/cpu.c| 5 +++--
src/cpu/cpu.h| 3 ++-
src/libxl/libxl_driver.c | 2 +-
src/qemu/qemu_domain.c | 5 +++--
src/qemu/qemu_driver.c | 6 +++---
src/qemu/qemu_migration_cookie.c | 3 ++-
tests/cputest.c | 5 +++--
11 files changed, 48 insertions(+), 20 deletions(-)
diff --git a/src/bhyve/bhyve_driver.c b/src/bhyve/bhyve_driver.c
index daa20bad40..fc57ccd504 100644
--- a/src/bhyve/bhyve_driver.c
+++ b/src/bhyve/bhyve_driver.c
@@ -1465,7 +1465,7 @@ bhyveConnectCompareCPU(virConnectPtr conn,
}
} else {
ret = virCPUCompareXML(caps->host.arch, caps->host.cpu,
- xmlDesc, failIncompatible);
+ xmlDesc, failIncompatible, false);
}
cleanup:
diff --git a/src/conf/cpu_conf.c b/src/conf/cpu_conf.c
index dea950ce68..40d8da4a8e 100644
--- a/src/conf/cpu_conf.c
+++ b/src/conf/cpu_conf.c
@@ -20,9 +20,11 @@
#include
+#include "configmake.h"
#include "virerror.h"
#include "viralloc.h"
#include "virbuffer.h"
+#include "virfile.h"
#include "cpu_conf.h"
#include "domain_conf.h"
#include "virstring.h"
@@ -281,7 +283,8 @@ virCPUDefCopy(const virCPUDef *cpu)
int
virCPUDefParseXMLString(const char *xml,
virCPUType type,
-virCPUDefPtr *cpu)
+virCPUDefPtr *cpu,
+bool validateXML)
{
xmlDocPtr doc = NULL;
xmlXPathContextPtr ctxt = NULL;
@@ -295,7 +298,7 @@ virCPUDefParseXMLString(const char *xml,
if (!(doc = virXMLParseStringCtxt(xml, _("(CPU_definition)"), &ctxt)))
goto cleanup;
-if (virCPUDefParseXML(ctxt, NULL, type, cpu) < 0)
+if (virCPUDefParseXML(ctxt, NULL, type, cpu, validateXML) < 0)
goto cleanup;
ret = 0;
@@ -323,7 +326,8 @@ int
virCPUDefParseXML(xmlXPathContextPtr ctxt,
const char *xpath,
virCPUType type,
- virCPUDefPtr *cpu)
+ virCPUDefPtr *cpu,
+ bool validateXML)
{
g_autoptr(virCPUDef) def = NULL;
g_autofree xmlNodePtr *nodes = NULL;
@@ -348,6 +352,22 @@ virCPUDefParseXML(xmlXPathContextPtr ctxt,
return -1;
}
+if (validateXML) {
+g_autofree char *schemafile = NULL;
+
+if (!(schemafile = virFileFindResource("cpu.rng",
+ abs_top_srcdir "/docs/schemas",
+ PKGDATADIR "/schemas")))
+return -1;
+
+if (virXMLValidateNodeAgainstSchema(schemafile, ctxt->doc,
+ctxt->node) < 0) {
+virReportError(VIR_ERR_XML_ERROR, "%s",
+ _("'cpu' element is not valid"));
+return -1;
+}
+}
+
def = virCPUDefNew();
if (type == VIR_CPU_TYPE_AUTO) {
@@ -1146,7 +1166,7 @@ virCPUDefListParse(const char **xmlCPUs,
if (!(doc = virXMLParseStringCtxt(xmlCPUs[i], _("(CPU_definition)"),
&ctxt)))
goto error;
-if (virCPUDefParseXML(ctxt, NULL, cpuType, &cpus[i]) < 0)
+if (virCPUDefParseXML(ctxt, NULL, cpuType, &cpus[i], false) < 0)
goto error;
xmlXPathFreeContext(ctxt);
diff --git a/src/conf/cpu_conf.h b/src/conf/cpu_conf.h
index 24c51e3a63..3ef14b7932 100644
--- a/src/conf/cpu_conf.h
+++ b/src/conf/cpu_conf.h
@@ -192,13 +192,15 @@ virCPUDefCopyWithoutModel(const virCPUDef *cpu);
int
virCPUDefParseXMLString(const char *xml,
virCPUType type,
-virCPUDefPtr *cpu);
+virCPUDefPtr *cpu,
+bool validateXML);
int
virCPUDefParseXML(xmlXPathContextPtr ctxt,
const char *xpath,
virCPUType mode,
- virCPUDefPtr *cpu);
+ virCPUDefPtr *cpu,
+ bool validateXML);
bool
virCPUDefIsEqual(virCPUDefPtr src,
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index 4d296f7bcb..2ae5329938 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -21583,7 +21583,8 @@ virDomainDefParseXML(xmlDocPtr xml,
}
VIR_FREE(nodes);
-if (virCPUDefParseXML(ctxt, "./cpu[1]", VIR_CPU_TYPE_GUEST, &def->cpu) < 0)
+if (virCPUDefParseXML(ctxt, "./cpu[1]", VIR_CPU_TYPE_GUEST, &def->cpu,
+ false) < 0)
goto error;
if (virDomainNumaDefParseXML(def->numa, ctxt) < 0)
diff --git a/src/cpu/cpu.c b/src/cpu/cpu.c
index 188c5d86b5..bf94811960 100644
--- a/src/cpu/cpu.c
+++ b/src/cpu/cpu.c
@@ -109,14 +109,15 @@ virCPUCompareResult
virCPUCompareXML(vir
[libvirt PATCH 2/9] schema: Move host cpu definition to cputypes.rng
Signed-off-by: Tim Wiederhake --- docs/schemas/capability.rng | 82 + docs/schemas/cputypes.rng | 74 + 2 files changed, 76 insertions(+), 80 deletions(-) diff --git a/docs/schemas/capability.rng b/docs/schemas/capability.rng index 68bdb29695..91a046eb48 100644 --- a/docs/schemas/capability.rng +++ b/docs/schemas/capability.rng @@ -3,6 +3,7 @@ http://relaxng.org/ns/structure/1.0"; datatypeLibrary="http://www.w3.org/2001/XMLSchema-datatypes";> + @@ -25,17 +26,7 @@ - - - - - - - - - - - + @@ -81,64 +72,6 @@ - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - @@ -567,17 +500,6 @@ - - - [a-zA-Z0-9\-_]+ - - - - - - - - diff --git a/docs/schemas/cputypes.rng b/docs/schemas/cputypes.rng index e6e82b5fd2..a2d4dbe9d1 100644 --- a/docs/schemas/cputypes.rng +++ b/docs/schemas/cputypes.rng @@ -299,4 +299,78 @@ + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + +[a-zA-Z0-9\-_]+ + + + + + + + + + + + + + + + + + + + + + -- 2.26.2
[libvirt PATCH 3/9] schema: Move guest cpu definition to cputypes.rng
Signed-off-by: Tim Wiederhake --- docs/schemas/cputypes.rng | 39 +++ docs/schemas/domaincommon.rng | 43 +-- 2 files changed, 40 insertions(+), 42 deletions(-) diff --git a/docs/schemas/cputypes.rng b/docs/schemas/cputypes.rng index a2d4dbe9d1..88f6904343 100644 --- a/docs/schemas/cputypes.rng +++ b/docs/schemas/cputypes.rng @@ -373,4 +373,43 @@ + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + diff --git a/docs/schemas/domaincommon.rng b/docs/schemas/domaincommon.rng index dedaf17b85..0c7a3a1385 100644 --- a/docs/schemas/domaincommon.rng +++ b/docs/schemas/domaincommon.rng @@ -44,7 +44,7 @@ - + @@ -5530,47 +5530,6 @@ - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - -
[libvirt PATCH 4/9] schema: Add schema for guest or host cpu definition
Signed-off-by: Tim Wiederhake --- docs/schemas/cpu.rng | 12 docs/schemas/meson.build | 1 + 2 files changed, 13 insertions(+) create mode 100644 docs/schemas/cpu.rng diff --git a/docs/schemas/cpu.rng b/docs/schemas/cpu.rng new file mode 100644 index 00..d1eb67430d --- /dev/null +++ b/docs/schemas/cpu.rng @@ -0,0 +1,12 @@ + +http://relaxng.org/ns/structure/1.0"; datatypeLibrary="http://www.w3.org/2001/XMLSchema-datatypes";> + + + + + + + + + + diff --git a/docs/schemas/meson.build b/docs/schemas/meson.build index 7fc7ff0503..bb6a48787f 100644 --- a/docs/schemas/meson.build +++ b/docs/schemas/meson.build @@ -1,6 +1,7 @@ docs_schema_files = [ 'basictypes.rng', 'capability.rng', + 'cpu.rng', 'cputypes.rng', 'domainbackup.rng', 'domaincaps.rng', -- 2.26.2
[libvirt PATCH 9/9] tests: Add tests for unknown elements and attributes in cpu defintion
Signed-off-by: Tim Wiederhake --- tests/cputest.c | 4 tests/cputestdata/x86_64-bogus-attribute.xml | 2 ++ tests/cputestdata/x86_64-bogus-element.xml | 3 +++ 3 files changed, 9 insertions(+) create mode 100644 tests/cputestdata/x86_64-bogus-attribute.xml create mode 100644 tests/cputestdata/x86_64-bogus-element.xml diff --git a/tests/cputest.c b/tests/cputest.c index 30a125c3da..2d00040497 100644 --- a/tests/cputest.c +++ b/tests/cputest.c @@ -1106,6 +1106,10 @@ mymain(void) } \ } while (0) +/* invalid xml */ +DO_TEST_COMPARE_FLAGS(VIR_ARCH_X86_64, "host", "bogus-element", VIR_CPU_COMPARE_ERROR, VIR_CONNECT_COMPARE_CPU_VALIDATE_XML); +DO_TEST_COMPARE_FLAGS(VIR_ARCH_X86_64, "host", "bogus-attribute", VIR_CPU_COMPARE_ERROR, VIR_CONNECT_COMPARE_CPU_VALIDATE_XML); + /* host to host comparison */ DO_TEST_COMPARE(VIR_ARCH_X86_64, "host", "host", VIR_CPU_COMPARE_IDENTICAL); DO_TEST_COMPARE(VIR_ARCH_X86_64, "host", "host-better", VIR_CPU_COMPARE_INCOMPATIBLE); diff --git a/tests/cputestdata/x86_64-bogus-attribute.xml b/tests/cputestdata/x86_64-bogus-attribute.xml new file mode 100644 index 00..073ee793e6 --- /dev/null +++ b/tests/cputestdata/x86_64-bogus-attribute.xml @@ -0,0 +1,2 @@ + + diff --git a/tests/cputestdata/x86_64-bogus-element.xml b/tests/cputestdata/x86_64-bogus-element.xml new file mode 100644 index 00..79f98bad18 --- /dev/null +++ b/tests/cputestdata/x86_64-bogus-element.xml @@ -0,0 +1,3 @@ + + + -- 2.26.2
[libvirt PATCH 8/9] tests: cpu: Allow passing flags to cpuTestLoadXML
Signed-off-by: Tim Wiederhake
---
tests/cputest.c | 45 ++---
1 file changed, 30 insertions(+), 15 deletions(-)
diff --git a/tests/cputest.c b/tests/cputest.c
index b40fd7f7f2..30a125c3da 100644
--- a/tests/cputest.c
+++ b/tests/cputest.c
@@ -68,7 +68,7 @@ static virQEMUDriver driver;
static virCPUDefPtr
-cpuTestLoadXML(virArch arch, const char *name)
+cpuTestLoadXML(virArch arch, const char *name, bool validate)
{
char *xml = NULL;
xmlDocPtr doc = NULL;
@@ -81,7 +81,7 @@ cpuTestLoadXML(virArch arch, const char *name)
if (!(doc = virXMLParseFileCtxt(xml, &ctxt)))
goto cleanup;
-virCPUDefParseXML(ctxt, NULL, VIR_CPU_TYPE_AUTO, &cpu, false);
+virCPUDefParseXML(ctxt, NULL, VIR_CPU_TYPE_AUTO, &cpu, validate);
cleanup:
xmlXPathFreeContext(ctxt);
@@ -203,12 +203,20 @@ cpuTestCompare(const void *arg)
virCPUDefPtr host = NULL;
virCPUDefPtr cpu = NULL;
virCPUCompareResult result;
+bool validate = !!(data->flags & VIR_CONNECT_COMPARE_CPU_VALIDATE_XML);
-if (!(host = cpuTestLoadXML(data->arch, data->host)) ||
-!(cpu = cpuTestLoadXML(data->arch, data->name)))
-goto cleanup;
+host = cpuTestLoadXML(data->arch, data->host, validate);
+cpu = cpuTestLoadXML(data->arch, data->name, validate);
+
+if (!host || !cpu) {
+if (validate)
+result = VIR_CPU_COMPARE_ERROR;
+else
+goto cleanup;
+} else {
+result = virCPUCompare(host->arch, host, cpu, false);
+}
-result = virCPUCompare(host->arch, host, cpu, false);
if (data->result == VIR_CPU_COMPARE_ERROR)
virResetLastError();
@@ -240,9 +248,10 @@ cpuTestGuestCPU(const void *arg)
virCPUCompareResult cmpResult;
g_auto(virBuffer) buf = VIR_BUFFER_INITIALIZER;
char *result = NULL;
+bool validate = !!(data->flags & VIR_CONNECT_COMPARE_CPU_VALIDATE_XML);
-if (!(host = cpuTestLoadXML(data->arch, data->host)) ||
-!(cpu = cpuTestLoadXML(data->arch, data->name)))
+if (!(host = cpuTestLoadXML(data->arch, data->host, validate)) ||
+!(cpu = cpuTestLoadXML(data->arch, data->name, validate)))
goto cleanup;
if (virCPUConvertLegacy(host->arch, cpu) < 0)
@@ -381,9 +390,10 @@ cpuTestUpdate(const void *arg)
virCPUDefPtr migHost = NULL;
virCPUDefPtr cpu = NULL;
char *result = NULL;
+bool validate = !!(data->flags & VIR_CONNECT_COMPARE_CPU_VALIDATE_XML);
-if (!(host = cpuTestLoadXML(data->arch, data->host)) ||
-!(cpu = cpuTestLoadXML(data->arch, data->name)))
+if (!(host = cpuTestLoadXML(data->arch, data->host, validate)) ||
+!(cpu = cpuTestLoadXML(data->arch, data->name, validate)))
goto cleanup;
if (!(migHost = virCPUCopyMigratable(data->arch, host)))
@@ -413,8 +423,9 @@ cpuTestHasFeature(const void *arg)
virCPUDefPtr host = NULL;
virCPUDataPtr hostData = NULL;
int result;
+bool validate = !!(data->flags & VIR_CONNECT_COMPARE_CPU_VALIDATE_XML);
-if (!(host = cpuTestLoadXML(data->arch, data->host)))
+if (!(host = cpuTestLoadXML(data->arch, data->host, validate)))
goto cleanup;
if (cpuEncode(host->arch, host, NULL, &hostData,
@@ -794,9 +805,10 @@ cpuTestUpdateLive(const void *arg)
virDomainCapsCPUModelsPtr hvModels = NULL;
virDomainCapsCPUModelsPtr models = NULL;
int ret = -1;
+bool validate = !!(data->flags & VIR_CONNECT_COMPARE_CPU_VALIDATE_XML);
cpuFile = g_strdup_printf("cpuid-%s-guest", data->host);
-if (!(cpu = cpuTestLoadXML(data->arch, cpuFile)))
+if (!(cpu = cpuTestLoadXML(data->arch, cpuFile, validate)))
goto cleanup;
enabledFile = g_strdup_printf("%s/cputestdata/%s-cpuid-%s-enabled.xml",
@@ -812,7 +824,7 @@ cpuTestUpdateLive(const void *arg)
goto cleanup;
expectedFile = g_strdup_printf("cpuid-%s-json", data->host);
-if (!(expected = cpuTestLoadXML(data->arch, expectedFile)))
+if (!(expected = cpuTestLoadXML(data->arch, expectedFile, validate)))
goto cleanup;
/* In case the host CPU signature does not exactly match any CPU model from
@@ -1018,10 +1030,13 @@ mymain(void)
VIR_FREE(testLabel); \
} while (0)
-#define DO_TEST_COMPARE(arch, host, cpu, result) \
+#define DO_TEST_COMPARE_FLAGS(arch, host, cpu, result, flags) \
DO_TEST(arch, cpuTestCompare, \
host "/" cpu " (" #result ")", \
-host, cpu, NULL, 0, result)
+host, cpu, NULL, flags, result)
+
+#define DO_TEST_COMPARE(arch, host, cpu, result) \
+DO_TEST_COMPARE_FLAGS(arch, host, cpu, result, 0)
#define DO_TEST_UPDATE_ONLY(arch, host, cpu) \
DO_TEST(arch, cpuTestUpdate, \
--
2.26.2
[libvirt PATCH 7/9] virsh: Add "validate" argument to [hypervisor-]cpu-compare
Signed-off-by: Tim Wiederhake
---
include/libvirt/libvirt-host.h | 1 +
src/bhyve/bhyve_driver.c | 7 +--
src/libxl/libxl_driver.c | 7 +--
src/qemu/qemu_driver.c | 18 +-
tools/virsh-host.c | 14 ++
5 files changed, 38 insertions(+), 9 deletions(-)
diff --git a/include/libvirt/libvirt-host.h b/include/libvirt/libvirt-host.h
index 6972834175..9e7840b9c2 100644
--- a/include/libvirt/libvirt-host.h
+++ b/include/libvirt/libvirt-host.h
@@ -754,6 +754,7 @@ typedef enum {
typedef enum {
VIR_CONNECT_COMPARE_CPU_FAIL_INCOMPATIBLE = (1 << 0), /* treat incompatible
CPUs as failure */
+VIR_CONNECT_COMPARE_CPU_VALIDATE_XML = (1 << 1), /* validate xml files */
} virConnectCompareCPUFlags;
int virConnectCompareCPU(virConnectPtr conn,
diff --git a/src/bhyve/bhyve_driver.c b/src/bhyve/bhyve_driver.c
index fc57ccd504..7dce3f8648 100644
--- a/src/bhyve/bhyve_driver.c
+++ b/src/bhyve/bhyve_driver.c
@@ -1442,14 +1442,17 @@ bhyveConnectCompareCPU(virConnectPtr conn,
int ret = VIR_CPU_COMPARE_ERROR;
virCapsPtr caps = NULL;
bool failIncompatible;
+bool validateXML;
-virCheckFlags(VIR_CONNECT_COMPARE_CPU_FAIL_INCOMPATIBLE,
+virCheckFlags(VIR_CONNECT_COMPARE_CPU_FAIL_INCOMPATIBLE |
+ VIR_CONNECT_COMPARE_CPU_VALIDATE_XML,
VIR_CPU_COMPARE_ERROR);
if (virConnectCompareCPUEnsureACL(conn) < 0)
goto cleanup;
failIncompatible = !!(flags & VIR_CONNECT_COMPARE_CPU_FAIL_INCOMPATIBLE);
+validateXML = !!(flags & VIR_CONNECT_COMPARE_CPU_VALIDATE_XML);
if (!(caps = bhyveDriverGetCapabilities(driver)))
goto cleanup;
@@ -1465,7 +1468,7 @@ bhyveConnectCompareCPU(virConnectPtr conn,
}
} else {
ret = virCPUCompareXML(caps->host.arch, caps->host.cpu,
- xmlDesc, failIncompatible, false);
+ xmlDesc, failIncompatible, validateXML);
}
cleanup:
diff --git a/src/libxl/libxl_driver.c b/src/libxl/libxl_driver.c
index 72864c2dc9..6d1f2edd54 100644
--- a/src/libxl/libxl_driver.c
+++ b/src/libxl/libxl_driver.c
@@ -6529,19 +6529,22 @@ libxlConnectCompareCPU(virConnectPtr conn,
libxlDriverConfigPtr cfg;
int ret = VIR_CPU_COMPARE_ERROR;
bool failIncompatible;
+bool validateXML;
-virCheckFlags(VIR_CONNECT_COMPARE_CPU_FAIL_INCOMPATIBLE,
+virCheckFlags(VIR_CONNECT_COMPARE_CPU_FAIL_INCOMPATIBLE |
+ VIR_CONNECT_COMPARE_CPU_VALIDATE_XML,
VIR_CPU_COMPARE_ERROR);
if (virConnectCompareCPUEnsureACL(conn) < 0)
return ret;
failIncompatible = !!(flags & VIR_CONNECT_COMPARE_CPU_FAIL_INCOMPATIBLE);
+validateXML = !!(flags & VIR_CONNECT_COMPARE_CPU_VALIDATE_XML);
cfg = libxlDriverConfigGet(driver);
ret = virCPUCompareXML(cfg->caps->host.arch, cfg->caps->host.cpu,
- xmlDesc, failIncompatible, false);
+ xmlDesc, failIncompatible, validateXML);
virObjectUnref(cfg);
return ret;
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index a7961ad3f0..a0c0a4c777 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -12227,20 +12227,23 @@ qemuConnectCompareCPU(virConnectPtr conn,
virQEMUDriverPtr driver = conn->privateData;
g_autoptr(virCPUDef) cpu = NULL;
bool failIncompatible;
+bool validateXML;
-virCheckFlags(VIR_CONNECT_COMPARE_CPU_FAIL_INCOMPATIBLE,
+virCheckFlags(VIR_CONNECT_COMPARE_CPU_FAIL_INCOMPATIBLE |
+ VIR_CONNECT_COMPARE_CPU_VALIDATE_XML,
VIR_CPU_COMPARE_ERROR);
if (virConnectCompareCPUEnsureACL(conn) < 0)
return VIR_CPU_COMPARE_ERROR;
failIncompatible = !!(flags & VIR_CONNECT_COMPARE_CPU_FAIL_INCOMPATIBLE);
+validateXML = !!(flags & VIR_CONNECT_COMPARE_CPU_VALIDATE_XML);
if (!(cpu = virQEMUDriverGetHostCPU(driver)))
return VIR_CPU_COMPARE_ERROR;
return virCPUCompareXML(driver->hostarch, cpu,
-xmlDesc, failIncompatible, false);
+xmlDesc, failIncompatible, validateXML);
}
@@ -12295,18 +12298,21 @@ qemuConnectCompareHypervisorCPU(virConnectPtr conn,
g_autoptr(virQEMUDriverConfig) cfg = virQEMUDriverGetConfig(driver);
g_autoptr(virQEMUCaps) qemuCaps = NULL;
bool failIncompatible;
+bool validateXML;
virCPUDefPtr hvCPU;
virCPUDefPtr cpu = NULL;
virArch arch;
virDomainVirtType virttype;
-virCheckFlags(VIR_CONNECT_COMPARE_CPU_FAIL_INCOMPATIBLE,
+virCheckFlags(VIR_CONNECT_COMPARE_CPU_FAIL_INCOMPATIBLE |
+ VIR_CONNECT_COMPARE_CPU_VALIDATE_XML,
VIR_CPU_COMPARE_ERROR);
if (virConnectCompareHypervisorCPUEnsureACL(conn) < 0)
goto cleanup;
failIncompatible = !!(flags & VIR_CONNE
[libvirt PATCH 5/9] util: Allow validation for single XML node
Signed-off-by: Tim Wiederhake
---
src/util/virxml.c | 15 +++
src/util/virxml.h | 6 ++
2 files changed, 21 insertions(+)
diff --git a/src/util/virxml.c b/src/util/virxml.c
index 5315d4ff6f..2ec526456f 100644
--- a/src/util/virxml.c
+++ b/src/util/virxml.c
@@ -1320,6 +1320,21 @@ virXMLValidateAgainstSchema(const char *schemafile,
}
+int
+virXMLValidateNodeAgainstSchema(const char *schemafile,
+xmlDocPtr doc,
+xmlNodePtr node)
+{
+xmlNodePtr root;
+int ret;
+
+root = xmlDocSetRootElement(doc, node);
+ret = virXMLValidateAgainstSchema(schemafile, doc);
+xmlDocSetRootElement(doc, root);
+return ret;
+}
+
+
void
virXMLValidatorFree(virXMLValidatorPtr validator)
{
diff --git a/src/util/virxml.h b/src/util/virxml.h
index 0301f15308..fd0d30fcec 100644
--- a/src/util/virxml.h
+++ b/src/util/virxml.h
@@ -212,6 +212,12 @@ virXMLValidatorValidate(virXMLValidatorPtr validator,
int
virXMLValidateAgainstSchema(const char *schemafile,
xmlDocPtr xml);
+
+int
+virXMLValidateNodeAgainstSchema(const char *schemafile,
+xmlDocPtr doc,
+xmlNodePtr node);
+
void
virXMLValidatorFree(virXMLValidatorPtr validator);
--
2.26.2
[libvirt PATCH 0/9] Make unknown XML elements fail CPU comparison
We currently ignore unknown elements in the CPU XML description, e.g. with virsh cpu-compare and hypervisor-cpu-compare. This makes '' (note the typo in "faeture") semantically identic to ''. No error is reported. This series adds checks for unrecognized attributes and elements in the "" element, catching this kind of mistake. V1: https://www.redhat.com/archives/libvir-list/2020-September/msg00933.html Changed: * Factored out Schema defintion of different cpu elements * Performing validation against actual schema * Add "--validate" option to virsh [hypervisor-]cpu-compare to opt-in to vali= dation * Drive-by: Unify quotation style in docs/schemas/* Tim Wiederhake (9): schema: Unify apostrophe and quotation mark usage schema: Move host cpu definition to cputypes.rng schema: Move guest cpu definition to cputypes.rng schema: Add schema for guest or host cpu definition util: Allow validation for single XML node cpu: Validate XML virsh: Add "validate" argument to [hypervisor-]cpu-compare tests: cpu: Allow passing flags to cpuTestLoadXML tests: Add tests for unknown elements and attributes in cpu defintion docs/schemas/basictypes.rng | 82 +-- docs/schemas/capability.rng | 466 ++- docs/schemas/cpu.rng | 12 + docs/schemas/cputypes.rng| 117 +++- docs/schemas/domain.rng | 10 +- docs/schemas/domainbackup.rng| 178 +++--- docs/schemas/domaincaps.rng | 236 docs/schemas/domaincheckpoint.rng| 62 +- docs/schemas/domaincommon.rng| 431 +++--- docs/schemas/domainsnapshot.rng | 104 ++-- docs/schemas/interface.rng | 6 +- docs/schemas/meson.build | 1 + docs/schemas/network.rng | 44 +- docs/schemas/networkcommon.rng | 26 +- docs/schemas/networkport.rng | 6 +- docs/schemas/nodedev.rng | 464 +++ docs/schemas/nwfilter.rng| 32 +- docs/schemas/nwfilterbinding.rng | 4 +- docs/schemas/secret.rng | 70 +-- docs/schemas/storagecommon.rng | 110 ++-- docs/schemas/storagepool.rng | 584 +-- docs/schemas/storagepoolcaps.rng | 64 +- docs/schemas/storagevol.rng | 150 ++--- include/libvirt/libvirt-host.h | 1 + src/bhyve/bhyve_driver.c | 7 +- src/conf/cpu_conf.c | 28 +- src/conf/cpu_conf.h | 6 +- src/conf/domain_conf.c | 3 +- src/cpu/cpu.c| 5 +- src/cpu/cpu.h| 3 +- src/libxl/libxl_driver.c | 7 +- src/qemu/qemu_domain.c | 5 +- src/qemu/qemu_driver.c | 18 +- src/qemu/qemu_migration_cookie.c | 3 +- src/util/virxml.c| 15 + src/util/virxml.h| 6 + tests/cputest.c | 52 +- tests/cputestdata/x86_64-bogus-attribute.xml | 2 + tests/cputestdata/x86_64-bogus-element.xml | 3 + tools/virsh-host.c | 14 + 40 files changed, 1773 insertions(+), 1664 deletions(-) create mode 100644 docs/schemas/cpu.rng create mode 100644 tests/cputestdata/x86_64-bogus-attribute.xml create mode 100644 tests/cputestdata/x86_64-bogus-element.xml --=20 2.26.2
Re: [PATCH v5 0/8] Configurable policy for handling deprecated interfaces
On Mon, Sep 21, 2020 at 02:54:15PM +0200, Peter Krempa wrote: > On Mon, Sep 21, 2020 at 13:45:14 +0100, Richard W.M. Jones wrote: > > Some general comments on using the patch: > > > > * For libguestfs I chose to add > > > > -compat deprecated-input=reject,deprecated-output=hide > > > > This is only enabled in developer builds of libguestfs when we > > are running qemu directly (not via libvirt). The patch for > > this is attached. > > > > * What's the point/difference in having reject vs crash? > > I'll be adding the following documentation for the qemu.conf entry in > libvirt controling the feature: > > +# The "reject" option is less harsh towards the VMs but some code paths > ignore > +# errors reported by qemu and thus it may not be obvious that a deprecated > +# command/field was used, thus it's suggested to use the "crash" option > instead. I'm not sure if libguestfs should use reject or crash. But since most of the benefit of this is going to be in detecting deprecated CLI parameters in future, reject should be sufficient for us. Rich. -- Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones Read my programming and virtualization blog: http://rwmj.wordpress.com virt-builder quickly builds VMs from scratch http://libguestfs.org/virt-builder.1.html
Re: [PATCH v5 0/8] Configurable policy for handling deprecated interfaces
On Mon, 14 Sep 2020 at 09:55, Markus Armbruster wrote: > > New option -compat lets you configure what to do when deprecated > interfaces get used. This is intended for testing users of the > management interfaces. It is experimental. > > -compat deprecated-input= configures what to do when > deprecated input is received. Available policies: > > * accept: Accept deprecated commands and arguments (default) > * reject: Reject them > * crash: Crash > > -compat deprecated-output= configures what to do when > deprecated output is sent. Available output policies: > > * accept: Emit deprecated command results and events (default) > * hide: Suppress them > > For now, -compat covers only deprecated syntactic aspects of QMP. We > may want to extend it to cover semantic aspects, CLI, and experimental > features. Some bikeshedding on option naming... If this only covers QMP, should we make the argument to -compat have a name that expresses that? eg deprecated-qmp-input, deprecated-qmp-output ? Also, it seems a bit repetitive to say 'deprecated' here all the time -- do you have a future use of -compat in mind which would be to adjust something that is *not* deprecated ? If not, maybe the 'deprecated' part should be in the option name rather than in every argument, eg -deprecation-compat qmp-input=crash,qmp-output=hide,cli-option=reject ? thanks -- PMM
Re: [PATCH v5 0/8] Configurable policy for handling deprecated interfaces
On Mon, Sep 21, 2020 at 13:45:14 +0100, Richard W.M. Jones wrote: > Some general comments on using the patch: > > * For libguestfs I chose to add > > -compat deprecated-input=reject,deprecated-output=hide > > This is only enabled in developer builds of libguestfs when we > are running qemu directly (not via libvirt). The patch for > this is attached. > > * What's the point/difference in having reject vs crash? I'll be adding the following documentation for the qemu.conf entry in libvirt controling the feature: +# The "reject" option is less harsh towards the VMs but some code paths ignore +# errors reported by qemu and thus it may not be obvious that a deprecated +# command/field was used, thus it's suggested to use the "crash" option instead.
Re: [PATCH v5 0/8] Configurable policy for handling deprecated interfaces
Some general comments on using the patch:
* For libguestfs I chose to add
-compat deprecated-input=reject,deprecated-output=hide
This is only enabled in developer builds of libguestfs when we
are running qemu directly (not via libvirt). The patch for
this is attached.
* What's the point/difference in having reject vs crash?
* I hope that by hiding deprecated QAPI fields we may detect
errors in libguestfs, but I suspect what'll happen is it'll
cause fall-back behaviour which might be harder to detect.
* Be *really* good to have this for command line parameters!
Notes on the attached patch:
* https://libguestfs.org/guestfs-building.1.html
* Simple test:
LIBGUESTFS_BACKEND=direct \
LIBGUESTFS_HV=~/d/qemu/build/x86_64-softmmu/qemu-system-x86_64 \
./run libguestfs-test-tool
* Run the full test suite:
LIBGUESTFS_HV=~/d/qemu/build/x86_64-softmmu/qemu-system-x86_64 \
make -k check-direct
Rich.
--
Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones
Read my programming and virtualization blog: http://rwmj.wordpress.com
Fedora Windows cross-compiler. Compile Windows programs, test, and
build Windows installers. Over 100 libraries supported.
http://fedoraproject.org/wiki/MinGW
>From 90df6dc8a3278800f9f9dc23f626df5fa00b5950 Mon Sep 17 00:00:00 2001
From: "Richard W.M. Jones"
Date: Mon, 21 Sep 2020 13:18:05 +0100
Subject: [PATCH] lib: direct: Pass qemu -compat to detect deprecated features.
In developer versions of libguestfs only, pass the qemu -compat option
which will reject deprecated qemu features, giving us early warning if
we are using something that may be removed in future. This does not
affect stable branch builds or old versions of qemu which did not have
this flag.
---
lib/guestfs-internal.h | 3 +++
lib/launch-direct.c| 11 +++
2 files changed, 14 insertions(+)
diff --git a/lib/guestfs-internal.h b/lib/guestfs-internal.h
index d7ec7215d..4ad1cd125 100644
--- a/lib/guestfs-internal.h
+++ b/lib/guestfs-internal.h
@@ -33,6 +33,9 @@
#include
+/* Is this a developer version of libguestfs? */
+#define IS_DEVELOPER_VERSION ((PACKAGE_VERSION_MINOR & 1) == 1)
+
/* Minimum required version of libvirt for the libvirt backend.
*
* This is also checked at runtime because you can dynamically link
diff --git a/lib/launch-direct.c b/lib/launch-direct.c
index b6ed9766f..3e42609ff 100644
--- a/lib/launch-direct.c
+++ b/lib/launch-direct.c
@@ -501,6 +501,17 @@ launch_direct (guestfs_h *g, void *datav, const char *arg)
if (guestfs_int_qemu_supports (g, data->qemu_data, "-enable-fips"))
flag ("-enable-fips");
+ /* In non-stable versions of libguestfs, pass the -compat option to
+ * qemu so we can look for potentially deprecated features.
+ */
+ if (IS_DEVELOPER_VERSION &&
+ guestfs_int_qemu_supports (g, data->qemu_data, "-compat")) {
+start_list ("-compat") {
+ append_list ("deprecated-input=reject");
+ append_list ("deprecated-output=hide");
+} end_list ();
+ }
+
/* Newer versions of qemu (from around 2009/12) changed the
* behaviour of monitors so that an implicit '-monitor stdio' is
* assumed if we are in -nographic mode and there is no other
--
2.28.0.rc2
[libvirt PATCH] virfirewalld: fix g_variant_get call
We need to pass pointer to `array`. Reported-by: Ján Tomko Signed-off-by: Pavel Hrdina --- src/util/virfirewalld.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/util/virfirewalld.c b/src/util/virfirewalld.c index 69c8b73da0..12448f0681 100644 --- a/src/util/virfirewalld.c +++ b/src/util/virfirewalld.c @@ -215,7 +215,7 @@ virFirewallDGetZones(char ***zones, size_t *nzones) NULL) < 0) return -1; -g_variant_get(reply, "(@as)", array); +g_variant_get(reply, "(@as)", &array); *zones = g_variant_dup_strv(array, nzones); return 0; -- 2.26.2
Re: [libvirt PATCH 07/14] src/util/virfirewalld: convert to use GLib DBus
On Mon, Sep 21, 2020 at 09:57:33AM +0200, Ján Tomko wrote:
> On a Thursday in 2020, Pavel Hrdina wrote:
> > Signed-off-by: Pavel Hrdina
> > ---
> > src/util/virfirewalld.c | 197
> > tests/meson.build | 4 +-
> > tests/networkxml2firewalltest.c | 39 ---
> > tests/virfirewalltest.c | 154 ++---
> > 4 files changed, 180 insertions(+), 214 deletions(-)
> >
> > diff --git a/src/util/virfirewalld.c b/src/util/virfirewalld.c
> > index c14a6b6e65..69c8b73da0 100644
> > --- a/src/util/virfirewalld.c
> > +++ b/src/util/virfirewalld.c
> > @@ -196,9 +195,9 @@ virFirewallDGetBackend(void)
> > int
> > virFirewallDGetZones(char ***zones, size_t *nzones)
> > {
> > -DBusConnection *sysbus = virDBusGetSystemBus();
> > -DBusMessage *reply = NULL;
> > -int ret = -1;
> > +GDBusConnection *sysbus = virGDBusGetSystemBus();
> > +g_autoptr(GVariant) reply = NULL;
> > +g_autoptr(GVariant) array = NULL;
> >
> > *nzones = 0;
> > *zones = NULL;
> > @@ -206,23 +205,20 @@ virFirewallDGetZones(char ***zones, size_t *nzones)
> > if (!sysbus)
> > return -1;
> >
> > -if (virDBusCallMethod(sysbus,
> > - &reply,
> > - NULL,
> > - VIR_FIREWALL_FIREWALLD_SERVICE,
> > - "/org/fedoraproject/FirewallD1",
> > - "org.fedoraproject.FirewallD1.zone",
> > - "getZones",
> > - NULL) < 0)
> > -goto cleanup;
> > +if (virGDBusCallMethod(sysbus,
> > + &reply,
> > + NULL,
> > + VIR_FIREWALL_FIREWALLD_SERVICE,
> > + "/org/fedoraproject/FirewallD1",
> > + "org.fedoraproject.FirewallD1.zone",
> > + "getZones",
> > + NULL) < 0)
> > +return -1;
> >
> > -if (virDBusMessageDecode(reply, "a&s", nzones, zones) < 0)
> > -goto cleanup;
> > +g_variant_get(reply, "(@as)", array);
>
> Throughout the series you're not checking return values of
> g_variant_get.
>
> I'm getting assertion errors with firewalld disabled:
> (process:156524): GLib-CRITICAL **: 09:56:49.398: g_variant_get_type:
> assertion 'value != NULL' failed
>
> (process:156524): GLib-CRITICAL **: 09:56:49.399:
> g_variant_type_is_subtype_of: assertion 'g_variant_type_check (type)' failed
>
> (process:156524): GLib-CRITICAL **: 09:56:49.399: g_variant_dup_strv:
> assertion 'g_variant_is_of_type (value, G_VARIANT_TYPE_STRING_ARRAY)' failed
So the issue here is that the g_variant_get should have been:
g_variant_get(reply, "(@as)", &array);
I'll post a patch shortly.
Pavel
signature.asc
Description: PGP signature
Re: Plans for the next release
[please don't top-post on technical mailing lists] On a Monday in 2020, Alexander Wels wrote: Which version would that be? 6.7.0? No, 6.7.0 is already out: https://www.redhat.com/archives/libvirt-announce/2020-September/msg0.html The next release is 6.8.0 Since 2.0.0 we follow a time-based versioning scheme: https://libvirt.org/downloads.html#numbering Jano signature.asc Description: PGP signature
[PATCH] util: support PCI passthrough net device stats collection
Collect PCI passthrough net device stats from kernel by netlink
API.
Currently, libvirt can not get PCI passthrough net device stats,
run command:
#virsh domifstat instance --interface=52:54:00:2d:b2:35
error: Failed to get interface stats instance 52:54:00:2d:b2:35
error: internal error: Interface name not provided
The PCI device(usually SR-IOV virtual function device) is detached
while it's used in PCI passthrough mode. And we can not parse this
device from /proc/net/dev any more.
In this patch, libvirt check net device is VF of not firstly, then
query virNetDevVFInterfaceStats(new API).
virNetDevVFInterfaceStats parses VFs info of all PFs, compares MAC
address until the two MAC addresses match.
'#ip -s link show' can get the same result. Instead of parsing the
output result, implement this feature by libnl API.
Notice that this feature deponds on driver of PF.
Test on Mellanox ConnectX-4 Lx, it works well.
Also test on Intel Corporation 82599ES, it works, but only get 0.
(ip-link command get the same result).
Signed-off-by: zhenwei pi
---
meson.build | 4 ++
src/libvirt_private.syms | 1 +
src/qemu/qemu_driver.c | 3 +
src/util/virnetdev.c | 158 +++
src/util/virnetdev.h | 5 ++
5 files changed, 171 insertions(+)
diff --git a/meson.build b/meson.build
index 24535a403c..e17da9e2b9 100644
--- a/meson.build
+++ b/meson.build
@@ -1392,6 +1392,10 @@ if not get_option('virtualport').disabled()
endif
endif
+if cc.has_header_symbol('linux/if_link.h', 'IFLA_VF_STATS')
+ conf.set('WITH_VF_STATS', 1)
+endif
+
if host_machine.system() == 'windows'
ole32_dep = cc.find_library('ole32')
oleaut32_dep = cc.find_library('oleaut32')
diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
index bdbe3431b8..bcc40b8d69 100644
--- a/src/libvirt_private.syms
+++ b/src/libvirt_private.syms
@@ -2585,6 +2585,7 @@ virNetDevSetRcvMulti;
virNetDevSetupControl;
virNetDevSysfsFile;
virNetDevValidateConfig;
+virNetDevVFInterfaceStats;
# util/virnetdevbandwidth.h
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index ae715c01d7..f554010c40 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -10196,6 +10196,9 @@ qemuDomainInterfaceStats(virDomainPtr dom,
if (virDomainNetGetActualType(net) == VIR_DOMAIN_NET_TYPE_VHOSTUSER) {
if (virNetDevOpenvswitchInterfaceStats(net->ifname, stats) < 0)
goto cleanup;
+} else if (virDomainNetGetActualType(net) == VIR_DOMAIN_NET_TYPE_HOSTDEV) {
+if (virNetDevVFInterfaceStats(&net->mac, stats) < 0)
+goto cleanup;
} else {
if (virNetDevTapInterfaceStats(net->ifname, stats,
!virDomainNetTypeSharesHostView(net)) <
0)
diff --git a/src/util/virnetdev.c b/src/util/virnetdev.c
index e1a4cc2bef..be9b8ce4a9 100644
--- a/src/util/virnetdev.c
+++ b/src/util/virnetdev.c
@@ -1489,6 +1489,9 @@ static struct nla_policy ifla_vf_policy[IFLA_VF_MAX+1] = {
.maxlen = sizeof(struct ifla_vf_mac) },
[IFLA_VF_VLAN] = { .type = NLA_UNSPEC,
.maxlen = sizeof(struct ifla_vf_vlan) },
+#if defined(WITH_VF_STATS)
+[IFLA_VF_STATS] = { .type = NLA_NESTED },
+#endif
};
@@ -2265,6 +2268,151 @@ virNetDevSetNetConfig(const char *linkdev, int vf,
return 0;
}
+#if defined(WITH_VF_STATS)
+static struct nla_policy ifla_vfstats_policy[IFLA_VF_STATS_MAX+1] = {
+[IFLA_VF_STATS_RX_PACKETS] = { .type = NLA_U64 },
+[IFLA_VF_STATS_TX_PACKETS] = { .type = NLA_U64 },
+[IFLA_VF_STATS_RX_BYTES]= { .type = NLA_U64 },
+[IFLA_VF_STATS_TX_BYTES]= { .type = NLA_U64 },
+[IFLA_VF_STATS_BROADCAST] = { .type = NLA_U64 },
+[IFLA_VF_STATS_MULTICAST] = { .type = NLA_U64 },
+};
+
+static int
+virNetDevParseVfStats(struct nlattr **tb, virMacAddrPtr mac,
+ virDomainInterfaceStatsPtr stats)
+{
+int ret = -1, len;
+struct ifla_vf_mac *vf_lladdr;
+struct nlattr *nla, *t[IFLA_VF_MAX+1];
+struct nlattr *stb[IFLA_VF_STATS_MAX+1];
+
+if (tb == NULL || mac == NULL || stats == NULL) {
+return -1;
+}
+
+if (!tb[IFLA_VFINFO_LIST])
+return -1;
+
+len = nla_len(tb[IFLA_VFINFO_LIST]);
+
+for (nla = nla_data(tb[IFLA_VFINFO_LIST]); nla_ok(nla, len);
+nla = nla_next(nla, &len)) {
+ret = nla_parse(t, IFLA_VF_MAX, nla_data(nla), nla_len(nla),
+ifla_vf_policy);
+if (ret < 0)
+return -1;
+
+if (t[IFLA_VF_MAC] == NULL) {
+continue;
+}
+
+vf_lladdr = nla_data(t[IFLA_VF_MAC]);
+if(virMacAddrCmpRaw(mac, vf_lladdr->mac)) {
+continue;
+}
+
+if (t[IFLA_VF_STATS]) {
+ret = nla_parse_nested(stb, IFLA_VF_STATS_MAX,
+t[IFLA_VF_STATS],
+ifla_vfstats_policy);
+if (ret < 0)
+
[PATCH] RFC: fix error message in virMigrate3 if connection is broken
Currently virDomainMigrate3 reports "this function is not supported by the
connection driver: virDomainMigrate3" if connection to destination for example
is broken. This is a bit misleading.
This is a result of we treat errors as unsupported feature in
VIR_DRV_SUPPORTS_FEATURE macro. So let's handle errors instead. In order to
keep logic clean as before there are new helper functions
virDrvSupportsFeature/virDrvNSupportsFeature. They allow to keep if/else if
structure of feature tests.
I guess all the other occurences of VIR_DRV_SUPPORTS_FEATURE need to be
replaced with one these new helper functions so that we detect error early and
not have issues similar to virDomainMigrate3. I'm going to fix the other
places if this RFC is approved.
---
src/datatypes.c | 70 +++
src/datatypes.h | 7 +
src/libvirt-domain.c | 76 +++-
3 files changed, 123 insertions(+), 30 deletions(-)
diff --git a/src/datatypes.c b/src/datatypes.c
index 1db38c5..3fb71f4 100644
--- a/src/datatypes.c
+++ b/src/datatypes.c
@@ -1257,3 +1257,73 @@ virAdmClientDispose(void *obj)
virObjectUnref(clt->srv);
}
+
+
+/*
+ * Tests if feature is supported by connection. If testing failed
+ * due to error then function returns true as well and set @err flag
+ * to true. Thus positive result should be checked for an error.
+ * If @err is already set to true then no checking is done and
+ * the function returns true immediately so that previous error
+ * is not overwritten.
+ *
+ * Returns:
+ * truefeature is supported or testing hit error
+ * false feature is not supported
+ */
+bool
+virDrvSupportsFeature(virConnectPtr conn,
+ virDrvFeature feature,
+ bool *err)
+{
+int rc;
+
+if (*err)
+return true;
+
+if (!conn->driver->connectSupportsFeature)
+return false;
+
+if ((rc = conn->driver->connectSupportsFeature(conn, feature)) < 0) {
+*err = true;
+return true;
+}
+
+return rc > 0 ? true : false;
+}
+
+
+/*
+ * Tests if feature is NOT supported by connection. If testing failed
+ * due to error then function returns true as well and set @err flag
+ * to true. Thus positive result should be checked for an error.
+ * If @err is already set to true then no checking is done and
+ * the function returns true immediately so that previous error
+ * is not overwritten.
+ *
+ * Note that this function is not merge negation of virDrvSupportsFeature.
+ *
+ * Returns:
+ * truefeature is not supported or testing hit error
+ * false feature is supported
+ */
+bool
+virDrvNSupportsFeature(virConnectPtr conn,
+ virDrvFeature feature,
+ bool *err)
+{
+int rc;
+
+if (*err)
+return true;
+
+if (!conn->driver->connectSupportsFeature)
+return true;
+
+if ((rc = conn->driver->connectSupportsFeature(conn, feature)) < 0) {
+*err = true;
+return true;
+}
+
+return rc > 0 ? false : true;
+}
diff --git a/src/datatypes.h b/src/datatypes.h
index ade3779..e468d92 100644
--- a/src/datatypes.h
+++ b/src/datatypes.h
@@ -871,3 +871,10 @@ int
virAdmConnectCloseCallbackDataRegister(virAdmConnectCloseCallbackDataPtr cbd
virFreeCallback freecb);
int virAdmConnectCloseCallbackDataUnregister(virAdmConnectCloseCallbackDataPtr
cbdata,
virAdmConnectCloseFunc cb);
+
+bool virDrvSupportsFeature(virConnectPtr conn,
+ virDrvFeature feature,
+ bool *err);
+bool virDrvNSupportsFeature(virConnectPtr conn,
+virDrvFeature feature,
+bool *err);
diff --git a/src/libvirt-domain.c b/src/libvirt-domain.c
index cde86c7..f4f626a 100644
--- a/src/libvirt-domain.c
+++ b/src/libvirt-domain.c
@@ -3846,6 +3846,7 @@ virDomainMigrate3(virDomainPtr domain,
const char *dname = NULL;
const char *dxml = NULL;
unsigned long long bandwidth = 0;
+bool err = false;
VIR_DOMAIN_DEBUG(domain, "dconn=%p, params=%p, nparms=%u flags=0x%x",
dconn, params, nparams, flags);
@@ -3878,18 +3879,20 @@ virDomainMigrate3(virDomainPtr domain,
}
if (flags & VIR_MIGRATE_OFFLINE) {
-if (!VIR_DRV_SUPPORTS_FEATURE(domain->conn->driver, domain->conn,
- VIR_DRV_FEATURE_MIGRATION_OFFLINE)) {
-virReportError(VIR_ERR_ARGUMENT_UNSUPPORTED, "%s",
- _("offline migration is not supported by "
- "the source host"));
+if (virDrvNSupportsFeature(domain->conn,
+ VIR_DRV_FEATURE_MIGRATION_OFFLINE, &err)) {
+if (!err)
+virReportError(VIR_ERR_ARGUMENT_UNSUPPORTED, "%s",
+
Re: Plans for the next release
Which version would that be? 6.7.0? On Mon, Sep 21, 2020 at 3:16 AM Jiri Denemark wrote: > We are getting close to the next release of libvirt. To aim for the > release on Oct 01 I suggest entering the freeze on Thursday Sep 24 and > tagging RC2 on Tuesday Sep 29. > > I hope this works for everyone. > > Jirka > >
[PULL v3 13/15] cphp: remove deprecated cpu-add command(s)
From: Igor Mammedov
These were deprecated since 4.0, remove both HMP and QMP variants.
Users should use device_add command instead. To get list of
possible CPUs and options, use 'info hotpluggable-cpus' HMP
or query-hotpluggable-cpus QMP command.
Signed-off-by: Igor Mammedov
Reviewed-by: Thomas Huth
Acked-by: Dr. David Alan Gilbert
Reviewed-by: Michal Privoznik
Acked-by: Cornelia Huck
Message-Id: <20200915120403.1074579-1-imamm...@redhat.com>
Reviewed-by: Michael S. Tsirkin
Signed-off-by: Michael S. Tsirkin
---
qapi/machine.json | 24 -
include/hw/boards.h | 1 -
include/hw/i386/pc.h| 1 -
include/monitor/hmp.h | 1 -
hw/core/machine-hmp-cmds.c | 12 -
hw/core/machine-qmp-cmds.c | 12 -
hw/i386/pc.c| 27 --
hw/i386/pc_piix.c | 1 -
hw/s390x/s390-virtio-ccw.c | 12 -
tests/qtest/cpu-plug-test.c | 100
tests/qtest/test-hmp.c | 1 -
docs/system/deprecated.rst | 25 +
hmp-commands.hx | 15 --
13 files changed, 21 insertions(+), 211 deletions(-)
diff --git a/qapi/machine.json b/qapi/machine.json
index 0ac1880e4a..d8ed096e9a 100644
--- a/qapi/machine.json
+++ b/qapi/machine.json
@@ -307,30 +307,6 @@
##
{ 'command': 'query-cpus-fast', 'returns': [ 'CpuInfoFast' ] }
-##
-# @cpu-add:
-#
-# Adds CPU with specified ID.
-#
-# @id: ID of CPU to be created, valid values [0..max_cpus)
-#
-# Features:
-# @deprecated: This command is deprecated. Use `device_add` instead.
-# See the `query-hotpluggable-cpus` command for details.
-#
-# Returns: Nothing on success
-#
-# Since: 1.5
-#
-# Example:
-#
-# -> { "execute": "cpu-add", "arguments": { "id": 2 } }
-# <- { "return": {} }
-#
-##
-{ 'command': 'cpu-add', 'data': {'id': 'int'},
- 'features': [ 'deprecated' ] }
-
##
# @MachineInfo:
#
diff --git a/include/hw/boards.h b/include/hw/boards.h
index 795910d01b..7abd5d889c 100644
--- a/include/hw/boards.h
+++ b/include/hw/boards.h
@@ -169,7 +169,6 @@ struct MachineClass {
void (*init)(MachineState *state);
void (*reset)(MachineState *state);
void (*wakeup)(MachineState *state);
-void (*hot_add_cpu)(MachineState *state, const int64_t id, Error **errp);
int (*kvm_type)(MachineState *machine, const char *arg);
void (*smp_parse)(MachineState *ms, QemuOpts *opts);
diff --git a/include/hw/i386/pc.h b/include/hw/i386/pc.h
index c14e14dfe0..be42fe3599 100644
--- a/include/hw/i386/pc.h
+++ b/include/hw/i386/pc.h
@@ -134,7 +134,6 @@ extern int fd_bootchk;
void pc_acpi_smi_interrupt(void *opaque, int irq, int level);
-void pc_hot_add_cpu(MachineState *ms, const int64_t id, Error **errp);
void pc_smp_parse(MachineState *ms, QemuOpts *opts);
void pc_guest_info_init(PCMachineState *pcms);
diff --git a/include/monitor/hmp.h b/include/monitor/hmp.h
index c986cfd28b..642e9e91f9 100644
--- a/include/monitor/hmp.h
+++ b/include/monitor/hmp.h
@@ -89,7 +89,6 @@ void hmp_chardev_add(Monitor *mon, const QDict *qdict);
void hmp_chardev_change(Monitor *mon, const QDict *qdict);
void hmp_chardev_remove(Monitor *mon, const QDict *qdict);
void hmp_chardev_send_break(Monitor *mon, const QDict *qdict);
-void hmp_cpu_add(Monitor *mon, const QDict *qdict);
void hmp_object_add(Monitor *mon, const QDict *qdict);
void hmp_object_del(Monitor *mon, const QDict *qdict);
void hmp_info_memdev(Monitor *mon, const QDict *qdict);
diff --git a/hw/core/machine-hmp-cmds.c b/hw/core/machine-hmp-cmds.c
index 3c47c5..f4092b98cc 100644
--- a/hw/core/machine-hmp-cmds.c
+++ b/hw/core/machine-hmp-cmds.c
@@ -46,18 +46,6 @@ void hmp_info_cpus(Monitor *mon, const QDict *qdict)
qapi_free_CpuInfoFastList(cpu_list);
}
-void hmp_cpu_add(Monitor *mon, const QDict *qdict)
-{
-int cpuid;
-Error *err = NULL;
-
-error_report("cpu_add is deprecated, please use device_add instead");
-
-cpuid = qdict_get_int(qdict, "id");
-qmp_cpu_add(cpuid, &err);
-hmp_handle_error(mon, err);
-}
-
void hmp_hotpluggable_cpus(Monitor *mon, const QDict *qdict)
{
Error *err = NULL;
diff --git a/hw/core/machine-qmp-cmds.c b/hw/core/machine-qmp-cmds.c
index 21551221ad..5362c80a18 100644
--- a/hw/core/machine-qmp-cmds.c
+++ b/hw/core/machine-qmp-cmds.c
@@ -284,18 +284,6 @@ HotpluggableCPUList *qmp_query_hotpluggable_cpus(Error
**errp)
return machine_query_hotpluggable_cpus(ms);
}
-void qmp_cpu_add(int64_t id, Error **errp)
-{
-MachineClass *mc;
-
-mc = MACHINE_GET_CLASS(current_machine);
-if (mc->hot_add_cpu) {
-mc->hot_add_cpu(current_machine, id, errp);
-} else {
-error_setg(errp, "Not supported");
-}
-}
-
void qmp_set_numa_node(NumaOptions *cmd, Error **errp)
{
if (!runstate_check(RUN_STATE_PRECONFIG)) {
diff --git a/hw/i386/pc.c b/hw/i386/pc.c
index b55369357e..5e1911fba0 100644
--- a/hw/i386/pc.c
+++ b/hw/i386/pc.c
@@ -777,32 +777,6 @@ void pc_smp_parse(MachineState *ms, Q
Re: [PATCH] util: fix non-null pointer parameter annotations
On Mon, 2020-09-21 at 09:59 +0100, Daniel P. Berrangé wrote: > An extra parameter was added to virQEMUBuildQemuImgKeySecretOpts in > > commit ecfc4094d832a23fb56e1825d799c93488c168d7 > Author: Daniel P. Berrangé > Date: Tue Sep 15 16:30:37 2020 +0100 > > storage: add support for qcow2 LUKS encryption > > but the non-null pointer annotations were not adjusted to take account. > > Signed-off-by: Daniel P. Berrangé > --- > src/util/virqemu.h | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) Reviewed-by: Andrea Bolognani -- Andrea Bolognani / Red Hat / Virtualization
[PATCH] util: fix non-null pointer parameter annotations
An extra parameter was added to virQEMUBuildQemuImgKeySecretOpts in commit ecfc4094d832a23fb56e1825d799c93488c168d7 Author: Daniel P. Berrangé Date: Tue Sep 15 16:30:37 2020 +0100 storage: add support for qcow2 LUKS encryption but the non-null pointer annotations were not adjusted to take account. Signed-off-by: Daniel P. Berrangé --- src/util/virqemu.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/util/virqemu.h b/src/util/virqemu.h index be14c04d51..2b33968158 100644 --- a/src/util/virqemu.h +++ b/src/util/virqemu.h @@ -63,4 +63,4 @@ void virQEMUBuildQemuImgKeySecretOpts(virBufferPtr buf, int format, virStorageEncryptionInfoDefPtr enc, const char *alias) -ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2) ATTRIBUTE_NONNULL(3); +ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(3) ATTRIBUTE_NONNULL(4); -- 2.26.2
Re: [PATCH v6 3/3] bhyve: soften requirements for slot 1
On Sun, Sep 20, 2020 at 07:21:15PM +0400, Roman Bogorodskiy wrote: > Currently, slot 1 is only allowed to be used by the LPC device. > Relax this requirement and allow to use slot 1 if it was explicitly > specified by the user for any other device type. In this case the LPC > device will have the next available address. > > If slot 1 was not used by the user, it'll be reserved for the LPC > device, even if it is not configured to make address assignment > consistent in case the LPC device becomes necessary (e.g. the user > adds a console or a video device which require LPC). > > Signed-off-by: Roman Bogorodskiy > --- > po/POTFILES.in| 1 - > src/bhyve/bhyve_device.c | 50 +-- > ...argv-addr-non-isa-controller-on-slot-1.xml | 1 + > tests/bhyvexml2argvtest.c | 2 +- > 4 files changed, 25 insertions(+), 29 deletions(-) Reviewed-by: Daniel P. Berrangé 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 v6 2/3] bhyve: support 'isa' controller for LPC
On Sun, Sep 20, 2020 at 07:21:14PM +0400, Roman Bogorodskiy wrote: > Support modeling of the 'isa' controller for bhyve. User can manually > define any PCI slot for the 'isa' controller, including PCI slot 1, > but other devices are not allowed to use this address. > > When domain configuration requires the 'isa' controller to be present, > automatically add it on domain post-parse stage. > > Now, as this controller is always available when needed, it's not > necessary to implicitly add it to the bhyve command line, so remove > bhyveBuildLPCArgStr(). > > Also, make bhyveDomainDefNeedsISAController() static as it's no longer > used outside of bhyve_domain.c. > > As more than one ISA controller is not supported by bhyve, > and multiple controllers with the same index are forbidden, > so forbid ISA controllers with non-zero index for bhyve. > > Signed-off-by: Roman Bogorodskiy > --- > src/bhyve/bhyve_command.c | 27 +++--- > src/bhyve/bhyve_device.c | 23 +--- > src/bhyve/bhyve_domain.c | 25 +++-- > src/bhyve/bhyve_domain.h | 2 -- > ...ml2argv-addr-isa-controller-on-slot-1.args | 10 ++ > ...2argv-addr-isa-controller-on-slot-1.ldargs | 3 ++ > ...xml2argv-addr-isa-controller-on-slot-1.xml | 26 ++ > ...l2argv-addr-isa-controller-on-slot-31.args | 10 ++ > ...argv-addr-isa-controller-on-slot-31.ldargs | 3 ++ > ...ml2argv-addr-isa-controller-on-slot-31.xml | 26 ++ > ...argv-addr-non-isa-controller-on-slot-1.xml | 23 > .../bhyvexml2argv-console.args| 2 +- > .../bhyvexml2argv-isa-controller.args | 10 ++ > .../bhyvexml2argv-isa-controller.ldargs | 3 ++ > .../bhyvexml2argv-isa-controller.xml | 24 + > ...bhyvexml2argv-isa-multiple-controllers.xml | 25 + > .../bhyvexml2argv-serial-grub-nocons.args | 2 +- > .../bhyvexml2argv-serial-grub.args| 2 +- > .../bhyvexml2argv-serial.args | 2 +- > .../bhyvexml2argvdata/bhyvexml2argv-uefi.args | 4 +-- > .../bhyvexml2argv-vnc-autoport.args | 4 +-- > .../bhyvexml2argv-vnc-vgaconf-io.args | 4 +-- > .../bhyvexml2argv-vnc-vgaconf-off.args| 4 +-- > .../bhyvexml2argv-vnc-vgaconf-on.args | 4 +-- > .../bhyvexml2argvdata/bhyvexml2argv-vnc.args | 4 +-- > tests/bhyvexml2argvtest.c | 5 +++ > ...l2xmlout-addr-isa-controller-on-slot-1.xml | 36 +++ > ...2xmlout-addr-isa-controller-on-slot-31.xml | 36 +++ > .../bhyvexml2xmlout-console.xml | 3 ++ > .../bhyvexml2xmlout-isa-controller.xml| 36 +++ > .../bhyvexml2xmlout-serial-grub-nocons.xml| 3 ++ > .../bhyvexml2xmlout-serial-grub.xml | 3 ++ > .../bhyvexml2xmlout-serial.xml| 3 ++ > .../bhyvexml2xmlout-vnc-autoport.xml | 3 ++ > .../bhyvexml2xmlout-vnc-vgaconf-io.xml| 3 ++ > .../bhyvexml2xmlout-vnc-vgaconf-off.xml | 3 ++ > .../bhyvexml2xmlout-vnc-vgaconf-on.xml| 3 ++ > .../bhyvexml2xmlout-vnc.xml | 3 ++ > tests/bhyvexml2xmltest.c | 3 ++ > 39 files changed, 378 insertions(+), 37 deletions(-) > create mode 100644 > tests/bhyvexml2argvdata/bhyvexml2argv-addr-isa-controller-on-slot-1.args > create mode 100644 > tests/bhyvexml2argvdata/bhyvexml2argv-addr-isa-controller-on-slot-1.ldargs > create mode 100644 > tests/bhyvexml2argvdata/bhyvexml2argv-addr-isa-controller-on-slot-1.xml > create mode 100644 > tests/bhyvexml2argvdata/bhyvexml2argv-addr-isa-controller-on-slot-31.args > create mode 100644 > tests/bhyvexml2argvdata/bhyvexml2argv-addr-isa-controller-on-slot-31.ldargs > create mode 100644 > tests/bhyvexml2argvdata/bhyvexml2argv-addr-isa-controller-on-slot-31.xml > create mode 100644 > tests/bhyvexml2argvdata/bhyvexml2argv-addr-non-isa-controller-on-slot-1.xml > create mode 100644 tests/bhyvexml2argvdata/bhyvexml2argv-isa-controller.args > create mode 100644 > tests/bhyvexml2argvdata/bhyvexml2argv-isa-controller.ldargs > create mode 100644 tests/bhyvexml2argvdata/bhyvexml2argv-isa-controller.xml > create mode 100644 > tests/bhyvexml2argvdata/bhyvexml2argv-isa-multiple-controllers.xml > create mode 100644 > tests/bhyvexml2xmloutdata/bhyvexml2xmlout-addr-isa-controller-on-slot-1.xml > create mode 100644 > tests/bhyvexml2xmloutdata/bhyvexml2xmlout-addr-isa-controller-on-slot-31.xml > create mode 100644 > tests/bhyvexml2xmloutdata/bhyvexml2xmlout-isa-controller.xml Reviewed-by: Daniel P. Berrangé 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 v6 1/3] conf: add 'isa' controller type
On Sun, Sep 20, 2020 at 07:21:13PM +0400, Roman Bogorodskiy wrote: > Introduce 'isa' controller type. In domain XML it looks this way: > > ... > > function='0x0'/> > > ... > > Currently, this is needed for the bhyve driver to allow choosing a > specific PCI address for that. In bhyve, this controller is used to > attach serial ports and a boot ROM. > > Signed-off-by: Roman Bogorodskiy > --- > docs/schemas/domaincommon.rng | 6 ++ > src/conf/domain_conf.c | 9 + > src/conf/domain_conf.h | 8 > src/qemu/qemu_command.c| 1 + > src/qemu/qemu_domain.c | 1 + > src/qemu/qemu_domain_address.c | 1 + > src/qemu/qemu_validate.c | 1 + > src/vbox/vbox_common.c | 1 + > 8 files changed, 28 insertions(+) Reviewed-by: Daniel P. Berrangé Regards, Daniel -- |: https://berrange.com -o-https://www.flickr.com/photos/dberrange :| |: https://libvirt.org -o-https://fstop138.berrange.com :| |: https://entangle-photo.org-o-https://www.instagram.com/dberrange :|
Re: [libvirt PATCH 07/14] src/util/virfirewalld: convert to use GLib DBus
On a Thursday in 2020, Pavel Hrdina wrote:
Signed-off-by: Pavel Hrdina
---
src/util/virfirewalld.c | 197
tests/meson.build | 4 +-
tests/networkxml2firewalltest.c | 39 ---
tests/virfirewalltest.c | 154 ++---
4 files changed, 180 insertions(+), 214 deletions(-)
diff --git a/src/util/virfirewalld.c b/src/util/virfirewalld.c
index c14a6b6e65..69c8b73da0 100644
--- a/src/util/virfirewalld.c
+++ b/src/util/virfirewalld.c
@@ -196,9 +195,9 @@ virFirewallDGetBackend(void)
int
virFirewallDGetZones(char ***zones, size_t *nzones)
{
-DBusConnection *sysbus = virDBusGetSystemBus();
-DBusMessage *reply = NULL;
-int ret = -1;
+GDBusConnection *sysbus = virGDBusGetSystemBus();
+g_autoptr(GVariant) reply = NULL;
+g_autoptr(GVariant) array = NULL;
*nzones = 0;
*zones = NULL;
@@ -206,23 +205,20 @@ virFirewallDGetZones(char ***zones, size_t *nzones)
if (!sysbus)
return -1;
-if (virDBusCallMethod(sysbus,
- &reply,
- NULL,
- VIR_FIREWALL_FIREWALLD_SERVICE,
- "/org/fedoraproject/FirewallD1",
- "org.fedoraproject.FirewallD1.zone",
- "getZones",
- NULL) < 0)
-goto cleanup;
+if (virGDBusCallMethod(sysbus,
+ &reply,
+ NULL,
+ VIR_FIREWALL_FIREWALLD_SERVICE,
+ "/org/fedoraproject/FirewallD1",
+ "org.fedoraproject.FirewallD1.zone",
+ "getZones",
+ NULL) < 0)
+return -1;
-if (virDBusMessageDecode(reply, "a&s", nzones, zones) < 0)
-goto cleanup;
+g_variant_get(reply, "(@as)", array);
Throughout the series you're not checking return values of
g_variant_get.
I'm getting assertion errors with firewalld disabled:
(process:156524): GLib-CRITICAL **: 09:56:49.398: g_variant_get_type: assertion
'value != NULL' failed
(process:156524): GLib-CRITICAL **: 09:56:49.399: g_variant_type_is_subtype_of:
assertion 'g_variant_type_check (type)' failed
(process:156524): GLib-CRITICAL **: 09:56:49.399: g_variant_dup_strv: assertion
'g_variant_is_of_type (value, G_VARIANT_TYPE_STRING_ARRAY)' failed
Jano
+*zones = g_variant_dup_strv(array, nzones);
-ret = 0;
- cleanup:
-virDBusMessageUnref(reply);
-return ret;
+return 0;
}
@@ -273,10 +269,10 @@ virFirewallDApplyRule(virFirewallLayer layer,
char **output)
{
const char *ipv = virFirewallLayerFirewallDTypeToString(layer);
-DBusConnection *sysbus = virDBusGetSystemBus();
-DBusMessage *reply = NULL;
-virError error;
-int ret = -1;
+GDBusConnection *sysbus = virGDBusGetSystemBus();
+g_autoptr(GVariant) message = NULL;
+g_autoptr(GVariant) reply = NULL;
+g_autoptr(virError) error = NULL;
if (!sysbus)
return -1;
@@ -287,23 +283,27 @@ virFirewallDApplyRule(virFirewallLayer layer,
virReportError(VIR_ERR_INTERNAL_ERROR,
_("Unknown firewall layer %d"),
layer);
-goto cleanup;
+return -1;
}
-if (virDBusCallMethod(sysbus,
- &reply,
- &error,
- VIR_FIREWALL_FIREWALLD_SERVICE,
- "/org/fedoraproject/FirewallD1",
- "org.fedoraproject.FirewallD1.direct",
- "passthrough",
- "sa&s",
- ipv,
- (int)argsLen,
- args) < 0)
-goto cleanup;
+if (VIR_ALLOC(error) < 0)
+return -1;
-if (error.level == VIR_ERR_ERROR) {
+message = g_variant_new("(s@as)",
+ipv,
+g_variant_new_strv((const char * const*)args,
argsLen));
+
+if (virGDBusCallMethod(sysbus,
+ &reply,
+ error,
+ VIR_FIREWALL_FIREWALLD_SERVICE,
+ "/org/fedoraproject/FirewallD1",
+ "org.fedoraproject.FirewallD1.direct",
+ "passthrough",
+ message) < 0)
+return -1;
+
+if (error->level == VIR_ERR_ERROR) {
/*
* As of firewalld-0.3.9.3-1.fc20.noarch the name and
* message fields in the error look like
@@ -331,22 +331,16 @@ virFirewallDApplyRule(virFirewallLayer layer,
*/
if (ignoreErrors) {
VIR_DEBUG("Ignoring error '%s': '%s'",
- error.str1, error.message);
+ error->str1, error->message);
} else {
-virReportErrorObject(&error);
-g
Plans for the next release
We are getting close to the next release of libvirt. To aim for the release on Oct 01 I suggest entering the freeze on Thursday Sep 24 and tagging RC2 on Tuesday Sep 29. I hope this works for everyone. Jirka
