Re: [libvirt PATCH 1/4] tests: virNumaGetPages: use g_new0 instead of VIR_ALLOC_N
On Wed, Sep 23, 2020 at 01:04:15 +0200, Ján Tomko wrote: > Signed-off-by: Ján Tomko > --- > tests/virnumamock.c | 16 +++- > 1 file changed, 3 insertions(+), 13 deletions(-) Reviewed-by: Peter Krempa
Re: [libvirt PATCH 2/4] tests: cpuTestLoadMultiXML: use g_new0 instead of VIR_ALLOC_N
On Wed, Sep 23, 2020 at 01:04:16 +0200, Ján Tomko wrote: > Signed-off-by: Ján Tomko > --- > tests/cputest.c | 4 +++- > 1 file changed, 3 insertions(+), 1 deletion(-) Reviewed-by: Peter Krempa
[libvirt PATCH v2] qemu: substitute missing model name for host-passthrough
From: Collin Walling
Before:
$ uname -m
s390x
$ cat passthrough-cpu.xml
$ virsh hypervisor-cpu-compare passthrough-cpu.xml
error: Failed to compare hypervisor CPU with passthrough-cpu.xml
error: internal error: unable to execute QEMU command 'query-cpu-model-comp
arison': Invalid parameter type for 'modelb.name', expected: string
After:
$ virsh hypervisor-cpu-compare passthrough-cpu.xml
CPU described in passthrough-cpu.xml is identical to the CPU provided by hy
pervisor on the host
Signed-off-by: Tim Wiederhake
---
src/qemu/qemu_driver.c | 9 +
1 file changed, 9 insertions(+)
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index ae715c01d7..1cecef01f7 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -12336,6 +12336,15 @@ qemuConnectCompareHypervisorCPU(virConnectPtr conn,
if (virCPUDefParseXMLString(xmlCPU, VIR_CPU_TYPE_AUTO, &cpu) < 0)
goto cleanup;
+if (!cpu->model) {
+if (cpu->mode == VIR_CPU_MODE_HOST_PASSTHROUGH) {
+cpu->model = g_strdup("host");
+} else {
+virReportError(VIR_ERR_INVALID_ARG, "%s",
+ _("cpu parameter is missing a model name"));
+goto cleanup;
+}
+}
ret = qemuConnectCPUModelComparison(qemuCaps, cfg->libDir,
cfg->user, cfg->group,
hvCPU, cpu, failIncompatible);
--
2.26.2
Re: [libvirt PATCH 3/5] rpc: socket: properly call virSetCloseExec
On Tue, Sep 22, 2020 at 11:32:31PM +0200, Ján Tomko wrote:
> cppcheck reports:
> style: Argument 'fd<0' to function virSetCloseExec is always 0 [knownArgument]
>
> Signed-off-by: Ján Tomko
> Fixes: 4b9919af4024a6fbc3d4ee996d8a4c27dbc44285
> ---
> src/rpc/virnetsocket.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/src/rpc/virnetsocket.c b/src/rpc/virnetsocket.c
> index ebdeadc4a0..f79a638775 100644
> --- a/src/rpc/virnetsocket.c
> +++ b/src/rpc/virnetsocket.c
> @@ -1388,7 +1388,7 @@ int virNetSocketDupFD(virNetSocketPtr sock, bool
> cloexec)
> }
> #ifndef F_DUPFD_CLOEXEC
> if (cloexec &&
> -virSetCloseExec(fd < 0)) {
> +virSetCloseExec(fd) < 0) {
Ewww, so IIUC we were setting close-exec on fd 0 every time here. Lucky
it didn't do anything worse.
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 3/4] tests: use g_new0 instead of VIR_ALLOC_N
On Wed, Sep 23, 2020 at 01:04:17 +0200, Ján Tomko wrote: > Signed-off-by: Ján Tomko > --- [...] > diff --git a/tests/securityselinuxtest.c b/tests/securityselinuxtest.c > index ae4b08b9d8..fd746d1ca1 100644 > --- a/tests/securityselinuxtest.c > +++ b/tests/securityselinuxtest.c > @@ -71,8 +71,7 @@ testBuildDomainDef(bool dynamic, > goto error; > > def->virtType = VIR_DOMAIN_VIRT_KVM; > -if (VIR_ALLOC_N(def->seclabels, 1) < 0) > -goto error; > +def->seclabels = g_new0(char, 1); 'def' is virDomainDefPtr thus 'def->seclabels' is not char but 'virSecurityLabelDefPtr *' The compiler didn't moan because it's a double pointer, but it certainly doens't have enough size nor the correct type. > > if (VIR_ALLOC(secdef) < 0) > goto error;
Re: [libvirt PATCH] virfirewalld: fix g_variant_get call
On Mon, Sep 21, 2020 at 02:41:58PM +0200, Pavel Hrdina wrote: > 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(-) I know it's to soon for ping but we should get this in before release and it's trivial. > 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 > signature.asc Description: PGP signature
Re: [libvirt PATCH] virgdbus: add DBus reply format check
On Mon, Sep 21, 2020 at 03:49:34PM +0200, Pavel Hrdina wrote:
> 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.
I know it's to soon for ping but we should get this in before release.
> 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,
>
Re: [libvirt PATCH] docs: Document global_{period, quota} schedinfo fields
On Fri, Sep 18, 2020 at 06:37:58PM +0200, Andrea Bolognani wrote: > These fields have existed for a very long time but they were > never documented in virsh(1). > > https://bugzilla.redhat.com/show_bug.cgi?id=1354391 > Signed-off-by: Andrea Bolognani > --- > docs/manpages/virsh.rst | 3 ++- > 1 file changed, 2 insertions(+), 1 deletion(-) Reviewed-by: Pavel Hrdina signature.asc Description: PGP signature
Re: [PATCH] libxl: use b_info->{acpi,acpi} when available
On Tue, Sep 22, 2020 at 09:41:00PM -0600, Jim Fehlig wrote:
> On 9/18/20 9:51 AM, Daniel P. Berrangé wrote:
> > On Fri, Sep 18, 2020 at 05:41:21PM +0200, Marek Marczykowski-Górecki wrote:
> > > On Fri, Sep 18, 2020 at 05:12:52PM +0200, Michal Prívozník wrote:
> > > > On 9/18/20 1:31 PM, Daniel P. Berrangé wrote:
> > > > > On Wed, Sep 16, 2020 at 11:09:31AM +0200, Michal Privoznik wrote:
> > > > > > On 9/10/20 6:18 AM, Marek Marczykowski-Górecki wrote:
> > > > > > > b_info->u.hvm.{acpi,apic} are deprecated. But also, on recent
> > > > > > > libxl
> > > > > > > version (4.14) the old one seems to be broken. While libxl part
> > > > > > > should
> > > > > > > be fixed too, update the usage here and at some point drop
> > > > > > > support for
> > > > > > > the old version.
> > > > > > > b_info->acpi was added in Xen 4.8
> > > > > > > b_info->apic was added in Xen 4.10
> > > > > > > Xen 4.10 is the oldest version that still has security support
> > > > > > > (until
> > > > > > > December 2020).
> > > > > > >
> > > > > > > Signed-off-by: Marek Marczykowski-Górecki
> > > > > > >
> > > > > > > ---
> > > > > > > src/libxl/libxl_conf.c | 13
> > > > > > > +
> > > > > > > tests/libxlxml2domconfigdata/basic-hvm.json | 4 ++--
> > > > > > > tests/libxlxml2domconfigdata/cpu-shares-hvm.json| 4 ++--
> > > > > > > .../libxlxml2domconfigdata/fullvirt-acpi-slic.json | 4 ++--
> > > > > > > .../fullvirt-cpuid-legacy-nest.json | 4 ++--
> > > > > > > tests/libxlxml2domconfigdata/fullvirt-cpuid.json| 4 ++--
> > > > > > > .../max-eventchannels-hvm.json | 4 ++--
> > > > > > > tests/libxlxml2domconfigdata/max-gntframes-hvm.json | 4 ++--
> > > > > > > tests/libxlxml2domconfigdata/moredevs-hvm.json | 4 ++--
> > > > > > > .../libxlxml2domconfigdata/variable-clock-hvm.json | 4 ++--
> > > > > > > .../vnuma-hvm-legacy-nest.json | 4 ++--
> > > > > > > tests/libxlxml2domconfigdata/vnuma-hvm.json | 4 ++--
> > > > > > > 12 files changed, 35 insertions(+), 22 deletions(-)
> > > > > >
> > > > > > This looks good to me.
> > > > > >
> > > > > > Reviewed-by: Michal Privoznik
> > > > > >
> > > > > > I'll wait a bit with pushing it though in case Jim wants to chime
> > > > > > in.
> > > > >
> > > > > This broke the build on Ubuntu 1804 due to tests failing
> > > > >
> > > > > TEST: libxlxml2domconfigtest
> > > > > .. 10 FAIL
> > > >
> > > > Oh, Ubuntu 18.04 has libxen-dev-4.9.2 and I'm not sure about FreeBSD,
> > > > but
> > > > probably something old too. So we can't use xen 4.10 APIs even though
> > > > it was
> > > > released 3 years ago.
> > > >
> > > > Unfortunately, we will have to support Ubuntu 18.04 for quite some time
> > > > because 20.04 was released only a while ago and we have this two year
> > > > transition period:
> > > >
> > > > https://libvirt.org/platforms.html
> > > >
> > > > Marek, are you okay with me reverting the patch?
> > >
> > > Technically, the driver code itself should work on both versions (there
> > > is an #ifdef for that). It's only an issue with tests, where we don't have
> > > #ifdef inside json files...
> > >
> > > One idea would be to duplicate those affected json files and pick the
> > > right one based on the libxenlight version, but it doesn't sound nice...
> > > Any alternative ideas?
> >
> > I think just duplicating the JSON files is the best solution because it
> > is simple, low overhead and keeps test coverage in both versions.
>
> I'm catching up on libvirt mail and could have missed it, but I didn't see a
> follow-up patch to fix the test.
>
> Marek, do you have time for it? If not I'll get to it tomorrow.
Sadly I'm pretty busy this week, I may find some time tomorrow but can't
promise it.
--
Best Regards,
Marek Marczykowski-Górecki
Invisible Things Lab
A: Because it messes up the order in which people normally read text.
Q: Why is top-posting such a bad thing?
signature.asc
Description: PGP signature
Re: [libvirt PATCH] meson: Include value of expensive_tests in summary
On Tue, Sep 22, 2020 at 03:18:20PM +0200, Andrea Bolognani wrote: > It's useful information to have available at a glance. > > Signed-off-by: Andrea Bolognani > --- > meson.build | 1 + > 1 file changed, 1 insertion(+) Reviewed-by: Pavel Hrdina signature.asc Description: PGP signature
Re: [libvirt PATCH] tests: Don't advertise VIR_TEST_EXPENSIVE to users
On Tue, Sep 22, 2020 at 03:18:49PM +0200, Andrea Bolognani wrote: > Right now, the logic that takes care of deciding whether expensive > tests should be run or not is not working correctly: more > specifically, it's not possible to use something like > > $ VIR_TEST_EXPENSIVE=1 ninja test > > to override the default choice, because in meson.build we always > pass an explicit value that overrides whatever is present in the > environment. > > We could implement logic to make this work properly, but that > would require some refactoring of our test infrastructure and is > arguably of little value given that running > > $ meson build -Dexpensive_tests=enabled > > is very fast, so let's just stop telling users about the variable > instead and call it a day. > > Signed-off-by: Andrea Bolognani > --- > docs/advanced-tests.rst | 8 +--- > meson_options.txt | 2 +- > tests/test-lib.sh | 2 +- > 3 files changed, 3 insertions(+), 9 deletions(-) Reviewed-by: Pavel Hrdina signature.asc Description: PGP signature
Re: [libvirt PATCH 1/9] Jailhouse driver: first commit with skeleton code
On Tue, Sep 22, 2020 at 07:08:00PM +0100, Daniel P. Berrangé wrote: > On Thu, Sep 17, 2020 at 04:23:51PM +0100, Daniel P. Berrangé wrote: > > From: Prakhar Bansal > > > > --- > > include/libvirt/virterror.h | 2 +- > > libvirt.spec.in | 7 + > > m4/virt-driver-jailhouse.m4 | 42 + > > meson.build | 4 + > > meson_options.txt| 1 + > > src/conf/domain_conf.c | 1 + > > src/conf/domain_conf.h | 1 + > > src/jailhouse/Makefile.inc.am| 21 +++ > > src/jailhouse/jailhouse_driver.c | 219 +++ > > src/jailhouse/jailhouse_driver.h | 23 +++ > > src/jailhouse/libvirtd_jailhouse.aug | 43 ++ > > src/jailhouse/meson.build| 48 ++ > > src/libvirt.c| 10 ++ > > src/meson.build | 1 + > > src/qemu/qemu_command.c | 1 + > > src/util/virerror.c | 1 + > > 16 files changed, 424 insertions(+), 1 deletion(-) > > create mode 100644 m4/virt-driver-jailhouse.m4 > > create mode 100644 src/jailhouse/Makefile.inc.am > > create mode 100644 src/jailhouse/jailhouse_driver.c > > create mode 100644 src/jailhouse/jailhouse_driver.h > > create mode 100644 src/jailhouse/libvirtd_jailhouse.aug > > create mode 100644 src/jailhouse/meson.build In addition all of the autotools related files should be removed as well because we use only meson: m4/virt-driver-jailhouse.m4 src/jailhouse/Makefile.inc.am Pavel signature.asc Description: PGP signature
Re: [libvirt PATCH 4/4] tests: use g_new0 instead of VIR_ALLOC
On Wed, Sep 23, 2020 at 01:04:18 +0200, Ján Tomko wrote: > Signed-off-by: Ján Tomko > --- Reviewed-by: Peter Krempa
Re: [PATCH v2 0/2] block: deprecate the sheepdog driver
On Tue, Sep 22, 2020 at 18:42:52 +0100, Daniel Berrange wrote: > On Tue, Sep 22, 2020 at 01:09:06PM -0400, Neal Gompa wrote: > > On Tue, Sep 22, 2020 at 12:16 PM Daniel P. Berrangé > > wrote: > > > > > > 2 years back I proposed dropping the sheepdog mailing list from the > > > MAINTAINERS file, but somehow the patch never got picked up: > > > > > > https://lists.gnu.org/archive/html/qemu-block/2018-03/msg01048.html > > > > > > So here I am with the same patch again. > > > > > > This time I go further and deprecate the sheepdog driver entirely. > > > See the rationale in the second patch commit message. > > > > > > Daniel P. Berrangé (2): > > > block: drop moderated sheepdog mailing list from MAINTAINERS file > > > block: deprecate the sheepdog block driver > > > > > > MAINTAINERS| 1 - > > > block/sheepdog.c | 15 +++ > > > configure | 5 +++-- > > > docs/system/deprecated.rst | 9 + > > > 4 files changed, 27 insertions(+), 3 deletions(-) > > > > > > -- > > > 2.26.2 > > > > > > > > > > I don't know of anyone shipping this other than Fedora, and I > > certainly don't use it there. > > > > Upstream looks like it's unmaintained now, with no commits in over two > > years: https://github.com/sheepdog/sheepdog/commits/master > > > > Can we also get a corresponding change to eliminate this from libvirt? > > This is only deprecation in QEMU, the feature still exists and is > intended to be as fully functional as in previous releases. > > Assuming QEMU actually deletes the feature at end of the deprecation > cycle, then libvirt would look at removing its own support. There are two sheepdog-related bits in libvirt which are IMO completely separate: 1) the blockdev backend handling for the 'sheepdog' protocol driver This is the one connected to qemu's deprecation, but until there is a qemu version where sheepdog wasn't deprecated/removed supported by libvirt we can't really do much about it. On the other hand it's just a few generators of arguments for -drive/-blockdev so the burden is very low. 2) the sheepdog storage driver This one is completely separate from qemu and we can decide to deprecate/delete it at our own schedule as it will not influence the ability to start VMs. The last non-housekeeping commit in that driver seems to be dated: Thu Jun 18 16:20:42 2015 Similarly the burden of keeping this around is low, but I would not bat an eye if we remove it right away even.
Re: [libvirt PATCH 2/9] Jailhouse driver: Implementation of ConnectOpen
On Thu, Sep 17, 2020 at 04:23:52PM +0100, Daniel P. Berrangé wrote:
> From: Prakhar Bansal
>
> ---
> include/libvirt/virterror.h | 1 +
> po/POTFILES.in | 2 +
> src/jailhouse/Makefile.inc.am | 34 ++-
> src/jailhouse/jailhouse.conf| 10 +
> src/jailhouse/jailhouse_api.c | 372
> src/jailhouse/jailhouse_api.h | 74 ++
> src/jailhouse/jailhouse_driver.c| 302 +-
> src/jailhouse/jailhouse_driver.h| 51
> src/jailhouse/meson.build | 1 +
> src/libvirt.c | 10 -
> src/remote/remote_daemon.c | 4 +
> src/remote/remote_daemon_dispatch.c | 3 +-
> 12 files changed, 783 insertions(+), 81 deletions(-)
> create mode 100644 src/jailhouse/jailhouse.conf
> create mode 100644 src/jailhouse/jailhouse_api.c
> create mode 100644 src/jailhouse/jailhouse_api.h
>
> diff --git a/include/libvirt/virterror.h b/include/libvirt/virterror.h
> index 97f2ac16d8..9f1bca2684 100644
> --- a/include/libvirt/virterror.h
> +++ b/include/libvirt/virterror.h
> @@ -137,6 +137,7 @@ typedef enum {
> VIR_FROM_TPM = 70, /* Error from TPM */
> VIR_FROM_BPF = 71, /* Error from BPF code */
> VIR_FROM_JAILHOUSE = 72,/* Error from Jailhouse driver */
> +
> # ifdef VIR_ENUM_SENTINELS
> VIR_ERR_DOMAIN_LAST
> # endif
Should be in the previous patch.
> diff --git a/src/jailhouse/Makefile.inc.am b/src/jailhouse/Makefile.inc.am
> index 02822b2ea1..324c3b1b16 100644
> --- a/src/jailhouse/Makefile.inc.am
> +++ b/src/jailhouse/Makefile.inc.am
This file shouldn't exist any more.
> diff --git a/src/jailhouse/jailhouse_api.c b/src/jailhouse/jailhouse_api.c
> new file mode 100644
> index 00..cda00b50e7
> --- /dev/null
> +++ b/src/jailhouse/jailhouse_api.c
> @@ -0,0 +1,372 @@
> +/*
> + * jailhouse_api.c: Jailhouse API
> + *
> + * Copyright (C) 2020 Prakhar Bansal
> + *
> + * This library is free software; you can redistribute it and/or
> + * modify it under the terms of the GNU Lesser General Public
> + * License as published by the Free Software Foundation; either
> + * version 2.1 of the License, or (at your option) any later version.
> + *
> + * This library is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
> + * Lesser General Public License for more details.
> + *
> + * You should have received a copy of the GNU Lesser General Public
> + * License along with this library. If not, see
> + *
Re: [libvirt PATCH 3/9] Jailhouse driver: Implementation of ConnectGetType
On Thu, Sep 17, 2020 at 04:23:53PM +0100, Daniel P. Berrangé wrote:
> From: Prakhar Bansal
>
> ---
> src/jailhouse/jailhouse_driver.c | 28
> 1 file changed, 16 insertions(+), 12 deletions(-)
>
> diff --git a/src/jailhouse/jailhouse_driver.c
> b/src/jailhouse/jailhouse_driver.c
> index ac9da4c85d..75bf41fc11 100644
> --- a/src/jailhouse/jailhouse_driver.c
> +++ b/src/jailhouse/jailhouse_driver.c
> @@ -32,8 +32,10 @@
> #include "viralloc.h"
> #include "virfile.h"
> #include "virlog.h"
> +#include "virutil.h"
> #include "vircommand.h"
> #include "virpidfile.h"
> +#include "access/viraccessapicheck.h"
>
> #define VIR_FROM_THIS VIR_FROM_JAILHOUSE
>
> @@ -241,16 +243,19 @@ jailhouseStateInitialize(bool privileged G_GNUC_UNUSED,
> static const char *
> jailhouseConnectGetType(virConnectPtr conn)
> {
> -UNUSED(conn);
> -return NULL;
> +if (virConnectGetTypeEnsureACL(conn) < 0)
> +return NULL;
>
> +return "JAILHOUSE";
> }
>
> static char *
> jailhouseConnectGetHostname(virConnectPtr conn)
> {
> -UNUSED(conn);
> -return NULL;
> +if (virConnectGetHostnameEnsureACL(conn) < 0)
> +return NULL;
> +
> +return virGetHostname();
> }
>
> static int
> @@ -263,7 +268,7 @@ jailhouseNodeGetInfo(virConnectPtr conn, virNodeInfoPtr
> info)
>
> static int
> jailhouseConnectListAllDomains(virConnectPtr conn,
> - virDomainPtr ** domain, unsigned int flags)
> + virDomainPtr **domain, unsigned int flags)
> {
> UNUSED(conn);
> UNUSED(domain);
> @@ -300,7 +305,6 @@ jailhouseDomainCreate(virDomainPtr domain)
> {
> UNUSED(domain);
> return -1;
> -
> }
Put these formatting fixes in the original patch that caused them.
>
> static int
> @@ -350,18 +354,18 @@ static virHypervisorDriver jailhouseHypervisorDriver = {
> .connectOpen = jailhouseConnectOpen,/* 6.3.0 */
> .connectClose = jailhouseConnectClose, /* 6.3.0 */
> .connectListAllDomains = jailhouseConnectListAllDomains,/* 6.3.0 */
> -.domainLookupByID = jailhouseDomainLookupByID, /* 6.3.0 */
> -.domainLookupByUUID = jailhouseDomainLookupByUUID, /* 6.3.0 */
> -.domainLookupByName = jailhouseDomainLookupByName, /* 6.3.0 */
> -.domainGetXMLDesc = jailhouseDomainGetXMLDesc, /* 6.3.0 */
> -.domainCreate = jailhouseDomainCreate, /* 6.3.0 */
> .connectGetType = jailhouseConnectGetType, /* 6.3.0 */
> .connectGetHostname = jailhouseConnectGetHostname, /* 6.3.0 */
> -.nodeGetInfo = jailhouseNodeGetInfo,/* 6.3.0 */
> +.domainCreate = jailhouseDomainCreate, /* 6.3.0 */
> .domainShutdown = jailhouseDomainShutdown, /* 6.3.0 */
> .domainDestroy = jailhouseDomainDestroy,/* 6.3.0 */
> .domainGetInfo = jailhouseDomainGetInfo,/* 6.3.0 */
> .domainGetState = jailhouseDomainGetState, /* 6.3.0 */
> +.domainLookupByID = jailhouseDomainLookupByID, /* 6.3.0 */
> +.domainLookupByUUID = jailhouseDomainLookupByUUID, /* 6.3.0 */
> +.domainLookupByName = jailhouseDomainLookupByName, /* 6.3.0 */
> +.domainGetXMLDesc = jailhouseDomainGetXMLDesc, /* 6.3.0 */
> +.nodeGetInfo = jailhouseNodeGetInfo,/* 6.3.0 */
> };
This re-ordering should be dropped, or done in the original patch
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 4/9] Jailhouse driver: Implementation of DomainCreate* callbacks
On Thu, Sep 17, 2020 at 04:23:54PM +0100, Daniel P. Berrangé wrote:
> From: Prakhar Bansal
>
> Implemented Jailhouse hypervisor APIs for cell
> load/start/shutdown/destroy.
> ---
> src/jailhouse/jailhouse_api.c| 100 --
> src/jailhouse/jailhouse_api.h| 29 +
> src/jailhouse/jailhouse_driver.c | 217 +--
> src/jailhouse/jailhouse_driver.h | 8 ++
> 4 files changed, 335 insertions(+), 19 deletions(-)
>
> diff --git a/src/jailhouse/jailhouse_api.c b/src/jailhouse/jailhouse_api.c
> index cda00b50e7..783903e939 100644
> --- a/src/jailhouse/jailhouse_api.c
> +++ b/src/jailhouse/jailhouse_api.c
> @@ -43,11 +43,14 @@
> #define JAILHOUSE_CELLS
> "/sys/devices/jailhouse/cells"
> #define MAX_JAILHOUSE_SYS_CONFIG_FILE_SIZE 1024*1024
> #define MAX_JAILHOUSE_CELL_CONFIG_FILE_SIZE 1024
> +#define MAX_JAILHOUSE_CELL_IMAGE_FILE_SIZE 64*1024*1024
>
>
> #define JAILHOUSE_ENABLE _IOW(0, 0, void *)
> #define JAILHOUSE_DISABLE _IO(0, 1)
> #define JAILHOUSE_CELL_CREATE _IOW(0, 2, virJailhouseCellCreate)
> +#define JAILHOUSE_CELL_LOAD_IOW(0, 3, virJailhouseCellLoad)
> +#define JAILHOUSE_CELL_START _IOW(0, 4, virJailhouseCellId)
> #define JAILHOUSE_CELL_DESTROY _IOW(0, 5, virJailhouseCellId)
>
> #define VIR_FROM_THIS VIR_FROM_JAILHOUSE
> @@ -68,8 +71,6 @@ int cell_match(const struct dirent *dirent);
>
> int createCell(const char *conf_file);
>
> -int destroyCell(virJailhouseCellId cell_id);
> -
> int getCellInfo(const unsigned int id,
> virJailhouseCellInfoPtr * cell_info);
>
> @@ -254,7 +255,7 @@ readSysfsCellString(const unsigned int id, const char
> *entry)
> }
>
> int
> -getCellInfo(const unsigned int id, virJailhouseCellInfoPtr * cell_info_ptr)
> +getCellInfo(const unsigned int id, virJailhouseCellInfoPtr *cell_info_ptr)
> {
> char *tmp;
>
> @@ -345,28 +346,105 @@ getJailhouseCellsInfo(void)
> }
>
> int
> -destroyCell(virJailhouseCellId cell_id)
> +loadImagesInCell(virJailhouseCellId cell_id, char **images, int num_images)
> +{
> + virJailhousePreloadImagePtr image;
> + virJailhouseCellLoadPtr cell_load;
> + g_autofree char *buffer = NULL;
> + unsigned int n;
> + int len = -1, err = -1;
> + VIR_AUTOCLOSE fd = -1;
> +
> +
> + if (VIR_ALLOC(cell_load) < 0) {
> +virReportError(VIR_ERR_INTERNAL_ERROR,
> + "%s",
> + _("Insufficient memory for cell load"));
> +return -1;
> + }
> +
> +
> + if (VIR_ALLOC_N(cell_load->image, num_images) < 0) {
> +virReportError(VIR_ERR_INTERNAL_ERROR,
> + "%s",
> + _("Insufficient memory for cell load images"));
> +return -1;
> + }
No need to check failure - these abort on OOM.
> +
> + cell_load->id = cell_id;
> + cell_load->num_preload_images = num_images;
> +
> + for (n = 0, image = cell_load->image; n < num_images; n++, image++) {
> +len = virFileReadAll(images[n], MAX_JAILHOUSE_CELL_IMAGE_FILE_SIZE,
> &buffer);
> +if (len < 0 || !buffer) {
> + virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
> +_("Failed to read the image file %s"),
> +images[n]);
> + return -1;
> +}
> +
> +image->source_address = (unsigned long)buffer;
> +image->size = len;
> +
> +// TODO(Prakhar): Add support for target address.
> +image->target_address = 0;
> + }
> +
> + fd = openDev();
Check error here.
> +
> + err = ioctl(fd, JAILHOUSE_CELL_LOAD, cell_load);
> + if (err) {
> + virReportSystemError(errno,
> +_("Loading cell images for %d failed"),
> +cell_id.id);
> + return -1;
> + }
> +
> + return 0;
> +}
> -destroyJailhouseCells(virJailhouseCellInfoPtr *cell_info_list G_GNUC_UNUSED)
> +destroyCell(virJailhouseCellId cell_id)
> {
> +int err = -1;
> +VIR_AUTOCLOSE fd = -1;
> +
> +fd = openDev();
Check error
>
> -/* Iterate over all cells in cell_info_list and destroy each cell */
> -// TODO: Not implemented yet.
> +err = ioctl(fd, JAILHOUSE_CELL_DESTROY, &cell_id);
> +if (err) {
> +virReportSystemError(errno,
> + _("Destroying cell %d failed"),
> + cell_id.id);
> +
> +return -1;
> +}
> +typedef struct _virJailhousePreloadImage virJailhousePreloadImage;
> +typedef virJailhousePreloadImage *virJailhousePreloadImagePtr;
> +
> +struct _virJailhousePreloadImage {
> +__u64 source_address;
uint64_t, etc
> +__u64 size;
> +__u64 target_address;
> +__u64 padding;
> +};
> +
> +typedef struct _virJailhouseCellLoad virJailhouseCellLoad;
> +typedef virJailhouseCellLoad *virJailhouseCellLoadPtr;
> +
> +struct _virJailhouseCellLoad {
> +str
Re: [libvirt PATCH 8/9] Jailhouse driver: Fixes for creation of cells, fetching cell info, disabling jailhouse hypervisor
On Thu, Sep 17, 2020 at 04:23:58PM +0100, Daniel P. Berrangé wrote: > From: Prakhar Bansal > > - Added xmlopt to the Jailhouse driver > - Added ACL check in ConnectOpen > --- > src/jailhouse/jailhouse_api.c| 48 +- > src/jailhouse/jailhouse_driver.c | 58 > 2 files changed, 61 insertions(+), 45 deletions(-) This patch essentially shoudn't exist. Use "git rebase -i master" to go back and put the fixes into the appropriate patch that has the original problem. 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 0/9] [GSoC]: Libvirt driver for Jailhouse
On Thu, Sep 17, 2020 at 04:23:50PM +0100, Daniel P. Berrangé wrote: > I'm sending this series on behalf of Prakhar Bansal who has been unable > to sucessfully send the whole series. GMail is blocking sending of > certain patches for unknown reasons. > > https://www.redhat.com/archives/libvir-list/2020-September/msg00309.html > > It is also available at > > https://github.com/pbansal2/libvirt/tree/jailhouse_driver I've had a look through the patches, and given some comments. Overall the driver looks like a pretty good initial effort. Most of the comments are more on the style / codiing patterns side. > > Prakhar Bansal (9): > Jailhouse driver: first commit with skeleton code > Jailhouse driver: Implementation of ConnectOpen > Jailhouse driver: Implementation of ConnectGetType > Jailhouse driver: Implementation of DomainCreate* callbacks > Jailhouse driver: Implementation of DomainInfo/State/List > Jailhouse driver: Implementation of DomainLookup* callbacks > Jailhouse driver: Implementation of DomainShutdown/Destroy callbacks > Jailhouse driver: Fixes for creation of cells, fetching cell info, > disabling jailhouse hypervisor > Jailhouse driver: Add events to events queue in > DomainCreate/Shutdown/Destroy > > include/libvirt/virterror.h | 1 + > libvirt.spec.in | 7 + > m4/virt-driver-jailhouse.m4 | 42 ++ > meson.build | 4 + > meson_options.txt| 1 + > po/POTFILES.in | 2 + > src/conf/domain_conf.c | 1 + > src/conf/domain_conf.h | 1 + > src/jailhouse/Makefile.inc.am| 47 ++ > src/jailhouse/jailhouse.conf | 10 + > src/jailhouse/jailhouse_api.c| 458 +++ > src/jailhouse/jailhouse_api.h| 103 > src/jailhouse/jailhouse_driver.c | 837 +++ > src/jailhouse/jailhouse_driver.h | 82 +++ > src/jailhouse/libvirtd_jailhouse.aug | 43 ++ > src/jailhouse/meson.build| 49 ++ > src/meson.build | 1 + > src/qemu/qemu_command.c | 1 + > src/remote/remote_daemon.c | 4 + > src/remote/remote_daemon_dispatch.c | 3 +- > src/util/virerror.c | 1 + > 21 files changed, 1697 insertions(+), 1 deletion(-) > create mode 100644 m4/virt-driver-jailhouse.m4 > create mode 100644 src/jailhouse/Makefile.inc.am > create mode 100644 src/jailhouse/jailhouse.conf > create mode 100644 src/jailhouse/jailhouse_api.c > create mode 100644 src/jailhouse/jailhouse_api.h > create mode 100644 src/jailhouse/jailhouse_driver.c > create mode 100644 src/jailhouse/jailhouse_driver.h > create mode 100644 src/jailhouse/libvirtd_jailhouse.aug > create mode 100644 src/jailhouse/meson.build > > -- > 2.26.2 > 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 3/9] Jailhouse driver: Implementation of ConnectGetType
On a Thursday in 2020, Daniel P. Berrangé wrote:
From: Prakhar Bansal
---
src/jailhouse/jailhouse_driver.c | 28
1 file changed, 16 insertions(+), 12 deletions(-)
diff --git a/src/jailhouse/jailhouse_driver.c b/src/jailhouse/jailhouse_driver.c
index ac9da4c85d..75bf41fc11 100644
--- a/src/jailhouse/jailhouse_driver.c
+++ b/src/jailhouse/jailhouse_driver.c
@@ -32,8 +32,10 @@
#include "viralloc.h"
#include "virfile.h"
#include "virlog.h"
+#include "virutil.h"
#include "vircommand.h"
#include "virpidfile.h"
+#include "access/viraccessapicheck.h"
#define VIR_FROM_THIS VIR_FROM_JAILHOUSE
@@ -241,16 +243,19 @@ jailhouseStateInitialize(bool privileged G_GNUC_UNUSED,
static const char *
jailhouseConnectGetType(virConnectPtr conn)
{
-UNUSED(conn);
-return NULL;
+if (virConnectGetTypeEnsureACL(conn) < 0)
+return NULL;
+return "JAILHOUSE";
}
There's no need to name it in all-caps.
"Jailhouse" or "jailhouse" will do
Jano
signature.asc
Description: PGP signature
Re: [libvirt PATCH] virfirewalld: fix g_variant_get call
On a Monday in 2020, Pavel Hrdina wrote: 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(-) Reviewed-by: Ján Tomko Tested-by: Ján Tomko Jano signature.asc Description: PGP signature
Re: [PATCH v2 0/2] block: deprecate the sheepdog driver
22.09.2020 21:11, Neal Gompa wrote: On Tue, Sep 22, 2020 at 1:43 PM Daniel P. Berrangé wrote: On Tue, Sep 22, 2020 at 01:09:06PM -0400, Neal Gompa wrote: On Tue, Sep 22, 2020 at 12:16 PM Daniel P. Berrangé wrote: 2 years back I proposed dropping the sheepdog mailing list from the MAINTAINERS file, but somehow the patch never got picked up: https://lists.gnu.org/archive/html/qemu-block/2018-03/msg01048.html So here I am with the same patch again. This time I go further and deprecate the sheepdog driver entirely. See the rationale in the second patch commit message. Daniel P. Berrangé (2): block: drop moderated sheepdog mailing list from MAINTAINERS file block: deprecate the sheepdog block driver MAINTAINERS| 1 - block/sheepdog.c | 15 +++ configure | 5 +++-- docs/system/deprecated.rst | 9 + 4 files changed, 27 insertions(+), 3 deletions(-) -- 2.26.2 I don't know of anyone shipping this other than Fedora, and I certainly don't use it there. Upstream looks like it's unmaintained now, with no commits in over two years: https://github.com/sheepdog/sheepdog/commits/master Can we also get a corresponding change to eliminate this from libvirt? This is only deprecation in QEMU, the feature still exists and is intended to be as fully functional as in previous releases. Assuming QEMU actually deletes the feature at end of the deprecation cycle, then libvirt would look at removing its own support. Does that stop us from deprecating it in libvirt though? I think not. Libvirt is not intended to support all Qemu features and I'm sure it doesn't. Amd it can safely deprecate features even if they are not-deprecated in Qemu. The only possible conflict is when Qemu wants to deprecate something that Libvirt wants to continue support for (or even can't live without). -- Best regards, Vladimir
[libvirt PATCH 1/3] esx: separate header and source file generation
Invoke the generator twice and introduce separate
meson targets for headers and C sources.
Signed-off-by: Ján Tomko
---
scripts/esx_vi_generator.py | 159
src/esx/meson.build | 31 +--
2 files changed, 114 insertions(+), 76 deletions(-)
diff --git a/scripts/esx_vi_generator.py b/scripts/esx_vi_generator.py
index 7929e1e682..2fac5152e5 100755
--- a/scripts/esx_vi_generator.py
+++ b/scripts/esx_vi_generator.py
@@ -1331,24 +1331,27 @@ additional_object_features = {
removed_object_features = {}
-if len(sys.argv) != 3:
-report_error("usage: %s srcdir builddir" % sys.argv[0])
+if len(sys.argv) != 4:
+report_error("usage: %s srcdir builddir header" % sys.argv[0])
input_filename = os.path.join(sys.argv[1], "esx/esx_vi_generator.input")
output_dirname = os.path.join(sys.argv[2], "esx")
+header = sys.argv[3] == "header"
-types_typedef = open_file(os.path.join(output_dirname,
"esx_vi_types.generated.typedef"))
-types_typeenum = open_file(os.path.join(output_dirname,
"esx_vi_types.generated.typeenum"))
-types_typetostring = open_file(os.path.join(output_dirname,
"esx_vi_types.generated.typetostring"))
-types_typefromstring = open_file(os.path.join(output_dirname,
"esx_vi_types.generated.typefromstring"))
-types_header = open_file(os.path.join(output_dirname,
"esx_vi_types.generated.h"))
-types_source = open_file(os.path.join(output_dirname,
"esx_vi_types.generated.c"))
-methods_header = open_file(os.path.join(output_dirname,
"esx_vi_methods.generated.h"))
-methods_source = open_file(os.path.join(output_dirname,
"esx_vi_methods.generated.c"))
-methods_macro = open_file(os.path.join(output_dirname,
"esx_vi_methods.generated.macro"))
-helpers_header = open_file(os.path.join(output_dirname, "esx_vi.generated.h"))
-helpers_source = open_file(os.path.join(output_dirname, "esx_vi.generated.c"))
+if header:
+types_typedef = open_file(os.path.join(output_dirname,
"esx_vi_types.generated.typedef"))
+types_typeenum = open_file(os.path.join(output_dirname,
"esx_vi_types.generated.typeenum"))
+types_typetostring = open_file(os.path.join(output_dirname,
"esx_vi_types.generated.typetostring"))
+types_typefromstring = open_file(os.path.join(output_dirname,
"esx_vi_types.generated.typefromstring"))
+types_header = open_file(os.path.join(output_dirname,
"esx_vi_types.generated.h"))
+methods_header = open_file(os.path.join(output_dirname,
"esx_vi_methods.generated.h"))
+helpers_header = open_file(os.path.join(output_dirname,
"esx_vi.generated.h"))
+else:
+types_source = open_file(os.path.join(output_dirname,
"esx_vi_types.generated.c"))
+methods_macro = open_file(os.path.join(output_dirname,
"esx_vi_methods.generated.macro"))
+methods_source = open_file(os.path.join(output_dirname,
"esx_vi_methods.generated.c"))
+helpers_source = open_file(os.path.join(output_dirname,
"esx_vi.generated.c"))
number = 0
@@ -1604,96 +1607,112 @@ for obj in managed_objects_by_name.values():
notice = "/* Generated by esx_vi_generator.py */\n\n\n\n"
-types_typedef.write(notice)
-types_typeenum.write(notice)
-types_typetostring.write(notice)
-types_typefromstring.write(notice)
-types_header.write(notice)
-types_source.write(notice)
-methods_header.write(notice)
-methods_source.write(notice)
-methods_macro.write(notice)
-helpers_header.write(notice)
-helpers_source.write(notice)
+if (header):
+types_typedef.write(notice)
+types_typeenum.write(notice)
+types_typetostring.write(notice)
+types_typefromstring.write(notice)
+types_header.write(notice)
+methods_header.write(notice)
+helpers_header.write(notice)
+else:
+types_source.write(notice)
+methods_macro.write(notice)
+methods_source.write(notice)
+helpers_source.write(notice)
# output enums
-types_typedef.write(separator +
-" * VI Enums\n" +
-" */\n\n")
+if header:
+types_typedef.write(separator +
+" * VI Enums\n" +
+" */\n\n")
names = sorted(enums_by_name.keys())
for name in names:
-types_typedef.write(enums_by_name[name].generate_typedef())
-types_typeenum.write(enums_by_name[name].generate_typeenum())
-types_typetostring.write(enums_by_name[name].generate_typetostring())
-types_typefromstring.write(enums_by_name[name].generate_typefromstring())
-types_header.write(enums_by_name[name].generate_header())
-types_source.write(enums_by_name[name].generate_source())
+if header:
+types_typedef.write(enums_by_name[name].generate_typedef())
+types_typeenum.write(enums_by_name[name].generate_typeenum())
+types_typetostring.write(enums_by_name[name].generate_typetostring())
+
types_typefromstring.write(enums_by_name[name].generate_typefromstring())
+types_header.write(enums_by_name[name].generate_header())
+else:
+types_source.write(enums_by_name[na
[libvirt PATCH 2/3] tests: esxutilstest: depend on esx_gen_headers
Sometimes parallel compilation randomly fails on platforms
that do not have many drivers enabled, like macOS:
In file included from ../tests/esxutilstest.c:13:
../src/esx/esx_vi_types.h:62:10: fatal error: 'esx_vi_types.generated.typedef'
file not found
^~~~
1 error generated.
List esx_gen_headers as a source to stop meson from building
it before the headers are generated.
https://gitlab.com/libvirt/libvirt/-/jobs/726039284
Signed-off-by: Ján Tomko
---
tests/meson.build | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/tests/meson.build b/tests/meson.build
index f4fbb25e66..8ae201fd69 100644
--- a/tests/meson.build
+++ b/tests/meson.build
@@ -355,8 +355,9 @@ if conf.has('WITH_BHYVE')
endif
if conf.has('WITH_ESX')
+ esxutilstest_sources = [ 'esxutilstest.c', esx_gen_headers ]
tests += [
-{ 'name': 'esxutilstest', 'include': [ esx_inc_dir ] },
+{ 'name': 'esxutilstest', 'sources': esxutilstest_sources, 'include': [
esx_inc_dir ] },
]
endif
--
2.26.2
[libvirt PATCH 3/3] sleep
meson build --werror -Dsystem=true \
-Ddriver-qemu=disabled -Ddriver-lxc=disabled \
-Ddriver-libxl=disabled -Ddriver-vbox=disabled \
-Ddriver-network=disabled -Ddriver-interface=disabled
---
scripts/esx_vi_generator.py | 3 +++
1 file changed, 3 insertions(+)
diff --git a/scripts/esx_vi_generator.py b/scripts/esx_vi_generator.py
index 2fac5152e5..fc29ec4a71 100755
--- a/scripts/esx_vi_generator.py
+++ b/scripts/esx_vi_generator.py
@@ -25,6 +25,7 @@
import sys
import os
import os.path
+import time
OCCURRENCE__REQUIRED_ITEM = "r"
@@ -43,6 +44,8 @@ autobind_names = set()
separator = "/* " + ("* " * 37) + "*\n"
+time.sleep(15)
+
def aligned(left, right, length=59):
return left.ljust(length, ' ') + right
--
2.26.2
[libvirt PATCH 0/3] esx: fix race when building tests
This should get rid of some of the random CI fails: https://gitlab.com/libvirt/libvirt/-/jobs/726039284 Ján Tomko (3): esx: separate header and source file generation tests: esxutilstest: depend on esx_gen_headers sleep scripts/esx_vi_generator.py | 162 src/esx/meson.build | 31 +-- tests/meson.build | 3 +- 3 files changed, 119 insertions(+), 77 deletions(-) -- 2.26.2
Re: [PATCH v2 0/3] improve pSeries NVDIMM support
On Tue, 2020-09-22 at 14:52 -0300, Daniel Henrique Barboza wrote: > On 9/22/20 11:56 AM, Andrea Bolognani wrote: > > On Tue, 2020-09-22 at 09:29 -0300, Daniel Henrique Barboza wrote: > > > Daniel Henrique Barboza (3): > > >conf, qemu: move qemuDomainNVDimmAlignSizePseries to domain_conf.c > > >domain_conf.c: auto-align pSeries NVDIMM in > > > virDomainMemoryDefPostParse() > > >NEWS.rst: update NVDIMM changes entry > > > > Looks good! > > > >Reviewed-by: Andrea Bolognani > > > > Can you please confirm your GitLab account is 'danielhb'? I would > > like to add you to the libvirt organization so that you can start > > pushing your own patches instead of relying on someone else doing > > that for you. > > Yes, 'danielhb' is my Gitlab username as well: > > https://gitlab.com/danielhb Great! I've added you to the group, you should be able to push patches to master now. Please make sure to pick up the R-bs before doing so. And, you know, always keep uncle Ben's wisdom in mind ;) -- Andrea Bolognani / Red Hat / Virtualization
Re: [libvirt PATCH 1/3] esx: separate header and source file generation
On Wed, Sep 23, 2020 at 11:17:43AM +0200, Ján Tomko wrote:
> Invoke the generator twice and introduce separate
> meson targets for headers and C sources.
>
> Signed-off-by: Ján Tomko
> ---
> scripts/esx_vi_generator.py | 159
> src/esx/meson.build | 31 +--
> 2 files changed, 114 insertions(+), 76 deletions(-)
>
> diff --git a/scripts/esx_vi_generator.py b/scripts/esx_vi_generator.py
> index 7929e1e682..2fac5152e5 100755
> --- a/scripts/esx_vi_generator.py
> +++ b/scripts/esx_vi_generator.py
> @@ -1331,24 +1331,27 @@ additional_object_features = {
>
> removed_object_features = {}
>
> -if len(sys.argv) != 3:
> -report_error("usage: %s srcdir builddir" % sys.argv[0])
> +if len(sys.argv) != 4:
> +report_error("usage: %s srcdir builddir header" % sys.argv[0])
>
> input_filename = os.path.join(sys.argv[1], "esx/esx_vi_generator.input")
> output_dirname = os.path.join(sys.argv[2], "esx")
> +header = sys.argv[3] == "header"
It would be nice to use argparse here but that's for a different series.
> -types_typedef = open_file(os.path.join(output_dirname,
> "esx_vi_types.generated.typedef"))
> -types_typeenum = open_file(os.path.join(output_dirname,
> "esx_vi_types.generated.typeenum"))
> -types_typetostring = open_file(os.path.join(output_dirname,
> "esx_vi_types.generated.typetostring"))
> -types_typefromstring = open_file(os.path.join(output_dirname,
> "esx_vi_types.generated.typefromstring"))
> -types_header = open_file(os.path.join(output_dirname,
> "esx_vi_types.generated.h"))
> -types_source = open_file(os.path.join(output_dirname,
> "esx_vi_types.generated.c"))
> -methods_header = open_file(os.path.join(output_dirname,
> "esx_vi_methods.generated.h"))
> -methods_source = open_file(os.path.join(output_dirname,
> "esx_vi_methods.generated.c"))
> -methods_macro = open_file(os.path.join(output_dirname,
> "esx_vi_methods.generated.macro"))
> -helpers_header = open_file(os.path.join(output_dirname,
> "esx_vi.generated.h"))
> -helpers_source = open_file(os.path.join(output_dirname,
> "esx_vi.generated.c"))
> +if header:
> +types_typedef = open_file(os.path.join(output_dirname,
> "esx_vi_types.generated.typedef"))
> +types_typeenum = open_file(os.path.join(output_dirname,
> "esx_vi_types.generated.typeenum"))
> +types_typetostring = open_file(os.path.join(output_dirname,
> "esx_vi_types.generated.typetostring"))
This file is included by `src/esx/esx_vi_types.c` as part of a switch
so we might consider it as source.
> +types_typefromstring = open_file(os.path.join(output_dirname,
> "esx_vi_types.generated.typefromstring"))
Similarly this file contains a lot of ifs and is included by
`src/esx/esx_vi_types.c` as source as well.
> +types_header = open_file(os.path.join(output_dirname,
> "esx_vi_types.generated.h"))
> +methods_header = open_file(os.path.join(output_dirname,
> "esx_vi_methods.generated.h"))
> +helpers_header = open_file(os.path.join(output_dirname,
> "esx_vi.generated.h"))
> +else:
> +types_source = open_file(os.path.join(output_dirname,
> "esx_vi_types.generated.c"))
> +methods_macro = open_file(os.path.join(output_dirname,
> "esx_vi_methods.generated.macro"))
> +methods_source = open_file(os.path.join(output_dirname,
> "esx_vi_methods.generated.c"))
> +helpers_source = open_file(os.path.join(output_dirname,
> "esx_vi.generated.c"))
>
>
> number = 0
> @@ -1604,96 +1607,112 @@ for obj in managed_objects_by_name.values():
>
> notice = "/* Generated by esx_vi_generator.py */\n\n\n\n"
>
> -types_typedef.write(notice)
> -types_typeenum.write(notice)
> -types_typetostring.write(notice)
> -types_typefromstring.write(notice)
> -types_header.write(notice)
> -types_source.write(notice)
> -methods_header.write(notice)
> -methods_source.write(notice)
> -methods_macro.write(notice)
> -helpers_header.write(notice)
> -helpers_source.write(notice)
> +if (header):
> +types_typedef.write(notice)
> +types_typeenum.write(notice)
> +types_typetostring.write(notice)
> +types_typefromstring.write(notice)
> +types_header.write(notice)
> +methods_header.write(notice)
> +helpers_header.write(notice)
> +else:
> +types_source.write(notice)
> +methods_macro.write(notice)
> +methods_source.write(notice)
> +helpers_source.write(notice)
>
>
> # output enums
> -types_typedef.write(separator +
> -" * VI Enums\n" +
> -" */\n\n")
> +if header:
> +types_typedef.write(separator +
> +" * VI Enums\n" +
> +" */\n\n")
>
> names = sorted(enums_by_name.keys())
>
> for name in names:
> -types_typedef.write(enums_by_name[name].generate_typedef())
> -types_typeenum.write(enums_by_name[name].generate_typeenum())
> -types_typetostring.write(enums_by_name[name].generate_typetostring())
> -types_typefromstring.write(enums_by_name[name].generate
Re: [libvirt PATCH 2/3] tests: esxutilstest: depend on esx_gen_headers
On Wed, Sep 23, 2020 at 11:17:44AM +0200, Ján Tomko wrote:
> Sometimes parallel compilation randomly fails on platforms
> that do not have many drivers enabled, like macOS:
>
> In file included from ../tests/esxutilstest.c:13:
> ../src/esx/esx_vi_types.h:62:10: fatal error:
> 'esx_vi_types.generated.typedef' file not found
> ^~~~
> 1 error generated.
>
> List esx_gen_headers as a source to stop meson from building
> it before the headers are generated.
>
> https://gitlab.com/libvirt/libvirt/-/jobs/726039284
>
> Signed-off-by: Ján Tomko
> ---
> tests/meson.build | 3 ++-
> 1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/tests/meson.build b/tests/meson.build
> index f4fbb25e66..8ae201fd69 100644
> --- a/tests/meson.build
> +++ b/tests/meson.build
> @@ -355,8 +355,9 @@ if conf.has('WITH_BHYVE')
> endif
>
> if conf.has('WITH_ESX')
> + esxutilstest_sources = [ 'esxutilstest.c', esx_gen_headers ]
>tests += [
> -{ 'name': 'esxutilstest', 'include': [ esx_inc_dir ] },
> +{ 'name': 'esxutilstest', 'sources': esxutilstest_sources, 'include': [
> esx_inc_dir ] },
>]
> endif
You can create esx_dep in src/esx/meson.build:
esx_dep = declare_dependency(
include_directories: esx_inc_dir,
sources: esx_gen_headers,
)
and use it in the test file:
{ 'name': 'esxutilstest', 'deps': [ esx_dep ] },
This way it's ensured that the include and headers are always used
together as a single dependency.
Pavel
signature.asc
Description: PGP signature
Re: [PATCH] Remove redundant check when storage pool is mounted
On Wed, Sep 23, 2020 at 11:34:57AM +0800, Yi Li wrote: > virFileComparePaths just return 0 or 1 after commit 7b48bb8 > so break while after virFileComparePaths return 1 > > Signed-off-by: Yi Li > --- > src/storage/storage_backend_fs.c | 8 ++-- > src/util/virfile.c | 1 - > 2 files changed, 2 insertions(+), 7 deletions(-) Reviewed-by: Daniel P. Berrangé and will push shortly. 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] util: support PCI passthrough net device stats collection
On Wed, Sep 23, 2020 at 10:32:54AM +0800, 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
> ---
> src/libvirt_private.syms | 1 +
> src/qemu/qemu_driver.c | 3 ++
> src/util/virnetdev.c | 137
> +++
> src/util/virnetdev.h | 5 ++
> 4 files changed, 146 insertions(+)
>
> 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..377f25aae7 100644
> --- a/src/util/virnetdev.c
> +++ b/src/util/virnetdev.c
> @@ -1489,6 +1489,7 @@ 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) },
> +[IFLA_VF_STATS] = { .type = NLA_NESTED },
> };
>
>
> @@ -2265,6 +2266,132 @@ virNetDevSetNetConfig(const char *linkdev, int vf,
> return 0;
> }
>
> +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)
> +return -1;
> +
> +stats->rx_bytes = nla_get_u64(stb[IFLA_VF_STATS_RX_BYTES]);
> +stats->tx_bytes = nla_get_u64(stb[IFLA_VF_STATS_TX_BYTES]);
> +stats-
Re: [PATCH v2 2/2] block: deprecate the sheepdog block driver
On 9/22/20 6:16 PM, Daniel P. Berrangé wrote:
> This thread from a little over a year ago:
>
> http://lists.wpkg.org/pipermail/sheepdog/2019-March/thread.html
>
> states that sheepdog is no longer actively developed. The only mentioned
> users are some companies who are said to have it for legacy reasons with
> plans to replace it by Ceph. There is talk about cutting out existing
> features to turn it into a simple demo of how to write a distributed
> block service. There is no evidence of anyone working on that idea:
>
> https://github.com/sheepdog/sheepdog/commits/master
>
> No real commits to git since Jan 2018, and before then just some minor
> technical debt cleanup..
>
> There is essentially no activity on the mailing list aside from
> patches to QEMU that get CC'd due to our MAINTAINERS entry.
>
> Fedora packages for sheepdog failed to build from upstream source
> because of the more strict linker that no longer merges duplicate
> global symbols. Fedora patches it to add the missing "extern"
> annotations and presumably other distros do to, but upstream source
> remains broken.
>
> There is only basic compile testing, no functional testing of the
> driver.
>
> Since there are no build pre-requisites the sheepdog driver is currently
> enabled unconditionally. This would result in configure issuing a
> deprecation warning by default for all users. Thus the configure default
> is changed to disable it, requiring users to pass --enable-sheepdog to
> build the driver.
>
> Signed-off-by: Daniel P. Berrangé
> ---
> block/sheepdog.c | 15 +++
> configure | 5 +++--
> docs/system/deprecated.rst | 9 +
> 3 files changed, 27 insertions(+), 2 deletions(-)
>
> diff --git a/block/sheepdog.c b/block/sheepdog.c
> index cbbebc1aaf..7f68bd6a1a 100644
> --- a/block/sheepdog.c
> +++ b/block/sheepdog.c
> @@ -242,6 +242,17 @@ typedef struct SheepdogInode {
> */
> #define FNV1A_64_INIT ((uint64_t)0xcbf29ce484222325ULL)
>
> +static void deprecation_warning(void)
> +{
> +static bool warned = false;
Checkpatch warns there is no need to initialize static bool to false,
otherwise:
Reviewed-by: Philippe Mathieu-Daudé
> +
> +if (!warned) {
> +warn_report("the sheepdog block driver is deprecated and will be "
> +"removed in a future release");
> +warned = true;
> +}
> +}
[...]
[libvirt PATCHv2 0/2] esx: fix race when building tests
v1: https://www.redhat.com/archives/libvir-list/2020-September/msg01196.html Ján Tomko (2): esx: separate header and source file generation tests: esxutilstest: depend on esx_gen_headers scripts/esx_vi_generator.py | 161 src/esx/meson.build | 34 ++-- tests/meson.build | 2 +- 3 files changed, 120 insertions(+), 77 deletions(-) -- 2.26.2
[libvirt PATCHv2 1/2] esx: separate header and source file generation
Invoke the generator twice and introduce separate
meson targets for headers and C sources.
Signed-off-by: Ján Tomko
---
scripts/esx_vi_generator.py | 161
src/esx/meson.build | 29 +--
2 files changed, 114 insertions(+), 76 deletions(-)
diff --git a/scripts/esx_vi_generator.py b/scripts/esx_vi_generator.py
index 7929e1e682..c9bd8f4920 100755
--- a/scripts/esx_vi_generator.py
+++ b/scripts/esx_vi_generator.py
@@ -1331,24 +1331,27 @@ additional_object_features = {
removed_object_features = {}
-if len(sys.argv) != 3:
-report_error("usage: %s srcdir builddir" % sys.argv[0])
+if len(sys.argv) != 4:
+report_error("usage: %s srcdir builddir header" % sys.argv[0])
input_filename = os.path.join(sys.argv[1], "esx/esx_vi_generator.input")
output_dirname = os.path.join(sys.argv[2], "esx")
+header = sys.argv[3] == "header"
-types_typedef = open_file(os.path.join(output_dirname,
"esx_vi_types.generated.typedef"))
-types_typeenum = open_file(os.path.join(output_dirname,
"esx_vi_types.generated.typeenum"))
-types_typetostring = open_file(os.path.join(output_dirname,
"esx_vi_types.generated.typetostring"))
-types_typefromstring = open_file(os.path.join(output_dirname,
"esx_vi_types.generated.typefromstring"))
-types_header = open_file(os.path.join(output_dirname,
"esx_vi_types.generated.h"))
-types_source = open_file(os.path.join(output_dirname,
"esx_vi_types.generated.c"))
-methods_header = open_file(os.path.join(output_dirname,
"esx_vi_methods.generated.h"))
-methods_source = open_file(os.path.join(output_dirname,
"esx_vi_methods.generated.c"))
-methods_macro = open_file(os.path.join(output_dirname,
"esx_vi_methods.generated.macro"))
-helpers_header = open_file(os.path.join(output_dirname, "esx_vi.generated.h"))
-helpers_source = open_file(os.path.join(output_dirname, "esx_vi.generated.c"))
+if header:
+types_typedef = open_file(os.path.join(output_dirname,
"esx_vi_types.generated.typedef"))
+types_typeenum = open_file(os.path.join(output_dirname,
"esx_vi_types.generated.typeenum"))
+types_header = open_file(os.path.join(output_dirname,
"esx_vi_types.generated.h"))
+methods_header = open_file(os.path.join(output_dirname,
"esx_vi_methods.generated.h"))
+helpers_header = open_file(os.path.join(output_dirname,
"esx_vi.generated.h"))
+else:
+types_typetostring = open_file(os.path.join(output_dirname,
"esx_vi_types.generated.typetostring"))
+types_typefromstring = open_file(os.path.join(output_dirname,
"esx_vi_types.generated.typefromstring"))
+types_source = open_file(os.path.join(output_dirname,
"esx_vi_types.generated.c"))
+methods_macro = open_file(os.path.join(output_dirname,
"esx_vi_methods.generated.macro"))
+methods_source = open_file(os.path.join(output_dirname,
"esx_vi_methods.generated.c"))
+helpers_source = open_file(os.path.join(output_dirname,
"esx_vi.generated.c"))
number = 0
@@ -1604,96 +1607,114 @@ for obj in managed_objects_by_name.values():
notice = "/* Generated by esx_vi_generator.py */\n\n\n\n"
-types_typedef.write(notice)
-types_typeenum.write(notice)
-types_typetostring.write(notice)
-types_typefromstring.write(notice)
-types_header.write(notice)
-types_source.write(notice)
-methods_header.write(notice)
-methods_source.write(notice)
-methods_macro.write(notice)
-helpers_header.write(notice)
-helpers_source.write(notice)
+if (header):
+types_typedef.write(notice)
+types_typeenum.write(notice)
+types_header.write(notice)
+methods_header.write(notice)
+helpers_header.write(notice)
+else:
+types_typetostring.write(notice)
+types_typefromstring.write(notice)
+types_source.write(notice)
+methods_macro.write(notice)
+methods_source.write(notice)
+helpers_source.write(notice)
# output enums
-types_typedef.write(separator +
-" * VI Enums\n" +
-" */\n\n")
+if header:
+types_typedef.write(separator +
+" * VI Enums\n" +
+" */\n\n")
names = sorted(enums_by_name.keys())
for name in names:
-types_typedef.write(enums_by_name[name].generate_typedef())
-types_typeenum.write(enums_by_name[name].generate_typeenum())
-types_typetostring.write(enums_by_name[name].generate_typetostring())
-types_typefromstring.write(enums_by_name[name].generate_typefromstring())
-types_header.write(enums_by_name[name].generate_header())
-types_source.write(enums_by_name[name].generate_source())
+if header:
+types_typedef.write(enums_by_name[name].generate_typedef())
+types_typeenum.write(enums_by_name[name].generate_typeenum())
+types_header.write(enums_by_name[name].generate_header())
+else:
+types_typetostring.write(enums_by_name[name].generate_typetostring())
+
types_typefromstring.write(enums_by_name[name].generate_typefromstring())
+types_source.write(enums_by_name[na
[libvirt PATCHv2 2/2] tests: esxutilstest: depend on esx_gen_headers
Sometimes parallel compilation randomly fails on platforms
that do not have many drivers enabled, like macOS:
In file included from ../tests/esxutilstest.c:13:
../src/esx/esx_vi_types.h:62:10: fatal error: 'esx_vi_types.generated.typedef'
file not found
#include "esx_vi_types.generated.typedef"
^~~~
1 error generated.
List esx_gen_headers as a source to stop meson from building
it before the headers are generated.
https://gitlab.com/libvirt/libvirt/-/jobs/726039284
Signed-off-by: Ján Tomko
---
src/esx/meson.build | 5 +
tests/meson.build | 2 +-
2 files changed, 6 insertions(+), 1 deletion(-)
diff --git a/src/esx/meson.build b/src/esx/meson.build
index 3e3d8591e5..2a7fcd54c9 100644
--- a/src/esx/meson.build
+++ b/src/esx/meson.build
@@ -81,3 +81,8 @@ else
endif
esx_inc_dir = include_directories('.')
+
+esx_dep = declare_dependency(
+ include_directories: esx_inc_dir,
+ sources: esx_gen_headers,
+)
diff --git a/tests/meson.build b/tests/meson.build
index f4fbb25e66..31e8d66c7a 100644
--- a/tests/meson.build
+++ b/tests/meson.build
@@ -356,7 +356,7 @@ endif
if conf.has('WITH_ESX')
tests += [
-{ 'name': 'esxutilstest', 'include': [ esx_inc_dir ] },
+{ 'name': 'esxutilstest', 'deps': [ esx_dep ] },
]
endif
--
2.26.2
Re: [libvirt PATCH] virgdbus: add DBus reply format check
On a Monday in 2020, Pavel Hrdina wrote: 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(-) Reviewed-by: Ján Tomko Jano signature.asc Description: PGP signature
Re: [libvirt PATCHv2 0/2] esx: fix race when building tests
On Wed, Sep 23, 2020 at 12:36:49PM +0200, Ján Tomko wrote: > v1: > https://www.redhat.com/archives/libvir-list/2020-September/msg01196.html > > Ján Tomko (2): > esx: separate header and source file generation > tests: esxutilstest: depend on esx_gen_headers Reviewed-by: Pavel Hrdina signature.asc Description: PGP signature
[libvirt PATCH] gdbus: fix virGDBusCallMethodWithFD stub for non-UNIX
Signed-off-by: Ján Tomko Fixes: a961d93768f18c28979ca2841832cd7278bf95b8 --- Pushed as a build fix. src/util/virgdbus.c | 1 + 1 file changed, 1 insertion(+) diff --git a/src/util/virgdbus.c b/src/util/virgdbus.c index 837c8faf1f..cd9ca8d5d6 100644 --- a/src/util/virgdbus.c +++ b/src/util/virgdbus.c @@ -310,6 +310,7 @@ virGDBusCallMethodWithFD(GDBusConnection *conn, int virGDBusCallMethodWithFD(GDBusConnection *conn G_GNUC_UNUSED, GVariant **reply G_GNUC_UNUSED, + const GVariantType *replyType G_GNUC_UNUSED, GUnixFDList **replyFD G_GNUC_UNUSED, virErrorPtr error G_GNUC_UNUSED, const char *busName G_GNUC_UNUSED, -- 2.26.2
Re: [PATCH v2] docs/system: clarify deprecation schedule
On Tue, Sep 15, 2020 at 04:07:34PM +0100, Stefan Hajnoczi wrote:
> The sentence explaining the deprecation schedule is ambiguous. Make it
> clear that a feature deprecated in the Nth release is guaranteed to
> remain available in the N+1th release. Removal can occur in the N+2nd
> release or later.
>
> As an example of this in action, see commit
> 25956af3fe5dd0385ad8017bc768a6afe41e2a74 ("block: Finish deprecation of
> 'qemu-img convert -n -o'"). The feature was deprecated in QEMU 4.2.0. It
> was present in the 5.0.0 release and removed in the 5.1.0 release.
>
> Suggested-by: Daniel P. Berrangé
> Signed-off-by: Stefan Hajnoczi
> ---
> v2:
> * Use Dan's suggested wording [Daniel Berrange]
> ---
> docs/system/deprecated.rst | 9 +
> 1 file changed, 5 insertions(+), 4 deletions(-)
Thanks, applied to my block tree:
https://github.com/stefanha/qemu/commits/block
Stefan
signature.asc
Description: PGP signature
Re: [libvirt PATCH 3/4] tests: use g_new0 instead of VIR_ALLOC_N
On a Wednesday in 2020, Peter Krempa wrote: On Wed, Sep 23, 2020 at 01:04:17 +0200, Ján Tomko wrote: Signed-off-by: Ján Tomko --- [...] diff --git a/tests/securityselinuxtest.c b/tests/securityselinuxtest.c index ae4b08b9d8..fd746d1ca1 100644 --- a/tests/securityselinuxtest.c +++ b/tests/securityselinuxtest.c @@ -71,8 +71,7 @@ testBuildDomainDef(bool dynamic, goto error; def->virtType = VIR_DOMAIN_VIRT_KVM; -if (VIR_ALLOC_N(def->seclabels, 1) < 0) -goto error; +def->seclabels = g_new0(char, 1); 'def' is virDomainDefPtr thus 'def->seclabels' is not char but 'virSecurityLabelDefPtr *' :neutral_face: :palm_tree: The compiler didn't moan because it's a double pointer, but it certainly doens't have enough size nor the correct type. Actually, the compiler did not even open this file. v2 coming up Jano if (VIR_ALLOC(secdef) < 0) goto error; signature.asc Description: PGP signature
[PATCH] docs: manpages: Strip table of contents from manpages
After meson conversion the man pages started to contain the table of contents. In autoconf we prevented this by a 'grep -v ::contents' in the command building the manpages. A more cultured solution is to strip out the 'contents' docutils element directly. Reported-by: Daniel P. Berrangé Signed-off-by: Peter Krempa --- docs/manpages/meson.build | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/docs/manpages/meson.build b/docs/manpages/meson.build index 3888bb8efe..7ed1d304a4 100644 --- a/docs/manpages/meson.build +++ b/docs/manpages/meson.build @@ -89,7 +89,8 @@ foreach data : docs_man_files man_file, input: rst_file, output: man_file, - command: [ rst2man_prog, '--strict', '@INPUT@', '@OUTPUT@' ], + # 'contents' element is the table of contents which is undesired in manpage + command: [ rst2man_prog, '--strip-elements-with-class', 'contents', '--strict', '@INPUT@', '@OUTPUT@' ], install: true, install_dir: mandir / 'man@0@'.format(data['section']), ) -- 2.26.2
Re: [PATCH] docs: manpages: Strip table of contents from manpages
On Wed, Sep 23, 2020 at 03:17:50PM +0200, Peter Krempa wrote: > After meson conversion the man pages started to contain the table of > contents. > > In autoconf we prevented this by a 'grep -v ::contents' in the command > building the manpages. > > A more cultured solution is to strip out the 'contents' docutils element > directly. > > Reported-by: Daniel P. Berrangé > Signed-off-by: Peter Krempa > --- > docs/manpages/meson.build | 3 ++- > 1 file changed, 2 insertions(+), 1 deletion(-) Reviewed-by: Pavel Hrdina signature.asc Description: PGP signature
Re: [PATCH] docs: manpages: Strip table of contents from manpages
On Wed, Sep 23, 2020 at 03:17:50PM +0200, Peter Krempa wrote: > After meson conversion the man pages started to contain the table of > contents. > > In autoconf we prevented this by a 'grep -v ::contents' in the command > building the manpages. > > A more cultured solution is to strip out the 'contents' docutils element > directly. Ah, very nice and clever ! > > Reported-by: Daniel P. Berrangé > Signed-off-by: Peter Krempa > --- > docs/manpages/meson.build | 3 ++- > 1 file changed, 2 insertions(+), 1 deletion(-) > > diff --git a/docs/manpages/meson.build b/docs/manpages/meson.build > index 3888bb8efe..7ed1d304a4 100644 > --- a/docs/manpages/meson.build > +++ b/docs/manpages/meson.build > @@ -89,7 +89,8 @@ foreach data : docs_man_files >man_file, >input: rst_file, >output: man_file, > - command: [ rst2man_prog, '--strict', '@INPUT@', '@OUTPUT@' ], > + # 'contents' element is the table of contents which is undesired in > manpage > + command: [ rst2man_prog, '--strip-elements-with-class', 'contents', > '--strict', '@INPUT@', '@OUTPUT@' ], >install: true, >install_dir: mandir / 'man@0@'.format(data['section']), > ) 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] apparmor: Allow /usr/libexec for libxl-save-helper and pygrub
On Wed, Sep 23, 2020 at 12:35 AM Jim Fehlig wrote:
>
> Like other distros, openSUSE Tumbleweed recently changed libexecdir from
> /usr/lib to /usr/libexec. Add it as an allowed path for libxl-save-helper
> and pygrub.
Hi Jim,
ack to the intention, but I think since this should use @libexecdir@ I think.
Or did anything change that this doesn't apply anymore ... in that
case I beg your pardon.
[1]:
https://libvirt.org/git/?p=libvirt.git;a=commit;h=5c8bd31c881e99261ac098e867a79b300440731a
> Signed-off-by: Jim Fehlig
> ---
>
> I considered including /usr/lib64, but I don't think any distros are
> installing xen libexecdir targets to /usr/lib64. Happy to include it
> if I'm wrong :-).
>
> src/security/apparmor/usr.sbin.libvirtd.in | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/src/security/apparmor/usr.sbin.libvirtd.in
> b/src/security/apparmor/usr.sbin.libvirtd.in
> index f2030764cd..bf4563e1e8 100644
> --- a/src/security/apparmor/usr.sbin.libvirtd.in
> +++ b/src/security/apparmor/usr.sbin.libvirtd.in
> @@ -86,8 +86,8 @@ profile libvirtd @sbindir@/libvirtd
> flags=(attach_disconnected) {
>/{usr/,}lib/udev/scsi_id PUx,
>/usr/{lib,lib64}/xen-common/bin/xen-toolstack PUx,
>/usr/{lib,lib64}/xen/bin/* Ux,
> - /usr/lib/xen-*/bin/libxl-save-helper PUx,
> - /usr/lib/xen-*/bin/pygrub PUx,
> + /usr/{lib,libexec}/xen-*/bin/libxl-save-helper PUx,
> + /usr/{lib,libexec}/xen-*/bin/pygrub PUx,
>/usr/{lib,lib64,lib/qemu,libexec}/vhost-user-gpu PUx,
>/usr/{lib,lib64,lib/qemu,libexec}/virtiofsd PUx,
>
> --
> 2.28.0
>
>
--
Christian Ehrhardt
Staff Engineer, Ubuntu Server
Canonical Ltd
Re: [PATCH] docs: manpages: Strip table of contents from manpages
On a Wednesday in 2020, Peter Krempa wrote: After meson conversion the man pages started to contain the table of contents. In autoconf we prevented this by a 'grep -v ::contents' in the command building the manpages. A more cultured solution is to strip out the 'contents' docutils element directly. Reported-by: Daniel P. Berrangé Signed-off-by: Peter Krempa --- docs/manpages/meson.build | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) Reviewed-by: Ján Tomko Jano signature.asc Description: PGP signature
[PATCH 06/13] virDomainSnapshotAlignDisks: Rename 'def' -> 'snapdef'
While this function resides in the snapshot config module the 'def'
variable is referencing the VM definition in most places. Change the
name to 'snapdef' to avoid ambiguity especially since we are also
dealing with the domain definition in this function.
Signed-off-by: Peter Krempa
---
src/conf/snapshot_conf.c | 42
1 file changed, 21 insertions(+), 21 deletions(-)
diff --git a/src/conf/snapshot_conf.c b/src/conf/snapshot_conf.c
index 160f2054a4..77f53c4a1d 100644
--- a/src/conf/snapshot_conf.c
+++ b/src/conf/snapshot_conf.c
@@ -645,7 +645,7 @@ virDomainSnapshotCompareDiskIndex(const void *a, const void
*b)
* dom->disks. If require_match, also ensure that there is no
* conflicting requests for both internal and external snapshots. */
int
-virDomainSnapshotAlignDisks(virDomainSnapshotDefPtr def,
+virDomainSnapshotAlignDisks(virDomainSnapshotDefPtr snapdef,
int default_snapshot,
bool require_match)
{
@@ -653,29 +653,29 @@ virDomainSnapshotAlignDisks(virDomainSnapshotDefPtr def,
size_t i;
int ndisks;
-if (!def->parent.dom) {
+if (!snapdef->parent.dom) {
virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
_("missing domain in snapshot"));
return -1;
}
-if (def->ndisks > def->parent.dom->ndisks) {
+if (snapdef->ndisks > snapdef->parent.dom->ndisks) {
virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
_("too many disk snapshot requests for domain"));
return -1;
}
/* Unlikely to have a guest without disks but technically possible. */
-if (!def->parent.dom->ndisks)
+if (!snapdef->parent.dom->ndisks)
return 0;
-if (!(map = virBitmapNew(def->parent.dom->ndisks)))
+if (!(map = virBitmapNew(snapdef->parent.dom->ndisks)))
return -1;
/* Double check requested disks. */
-for (i = 0; i < def->ndisks; i++) {
-virDomainSnapshotDiskDefPtr disk = &def->disks[i];
-int idx = virDomainDiskIndexByName(def->parent.dom, disk->name, false);
+for (i = 0; i < snapdef->ndisks; i++) {
+virDomainSnapshotDiskDefPtr disk = &snapdef->disks[i];
+int idx = virDomainDiskIndexByName(snapdef->parent.dom, disk->name,
false);
int disk_snapshot;
if (idx < 0) {
@@ -693,7 +693,7 @@ virDomainSnapshotAlignDisks(virDomainSnapshotDefPtr def,
ignore_value(virBitmapSetBit(map, idx));
disk->idx = idx;
-disk_snapshot = def->parent.dom->disks[idx]->snapshot;
+disk_snapshot = snapdef->parent.dom->disks[idx]->snapshot;
if (!disk->snapshot) {
if (disk_snapshot &&
(!require_match ||
@@ -721,44 +721,44 @@ virDomainSnapshotAlignDisks(virDomainSnapshotDefPtr def,
disk->src->path, disk->name);
return -1;
}
-if (STRNEQ(disk->name, def->parent.dom->disks[idx]->dst)) {
+if (STRNEQ(disk->name, snapdef->parent.dom->disks[idx]->dst)) {
VIR_FREE(disk->name);
-disk->name = g_strdup(def->parent.dom->disks[idx]->dst);
+disk->name = g_strdup(snapdef->parent.dom->disks[idx]->dst);
}
}
/* Provide defaults for all remaining disks. */
-ndisks = def->ndisks;
-if (VIR_EXPAND_N(def->disks, def->ndisks,
- def->parent.dom->ndisks - def->ndisks) < 0)
+ndisks = snapdef->ndisks;
+if (VIR_EXPAND_N(snapdef->disks, snapdef->ndisks,
+ snapdef->parent.dom->ndisks - snapdef->ndisks) < 0)
return -1;
-for (i = 0; i < def->parent.dom->ndisks; i++) {
+for (i = 0; i < snapdef->parent.dom->ndisks; i++) {
virDomainSnapshotDiskDefPtr disk;
if (virBitmapIsBitSet(map, i))
continue;
-disk = &def->disks[ndisks++];
+disk = &snapdef->disks[ndisks++];
disk->src = virStorageSourceNew();
-disk->name = g_strdup(def->parent.dom->disks[i]->dst);
+disk->name = g_strdup(snapdef->parent.dom->disks[i]->dst);
disk->idx = i;
/* Don't snapshot empty drives */
-if (virStorageSourceIsEmpty(def->parent.dom->disks[i]->src))
+if (virStorageSourceIsEmpty(snapdef->parent.dom->disks[i]->src))
disk->snapshot = VIR_DOMAIN_SNAPSHOT_LOCATION_NONE;
else
-disk->snapshot = def->parent.dom->disks[i]->snapshot;
+disk->snapshot = snapdef->parent.dom->disks[i]->snapshot;
disk->src->type = VIR_STORAGE_TYPE_FILE;
if (!disk->snapshot)
disk->snapshot = default_snapshot;
}
-qsort(&def->disks[0], def->ndisks, sizeof(def->disks[0]),
+qsort(&snapdef->disks[0], snapdef->ndisks, sizeof(snapdef->disks[0]),
virDomainSnapshotCompareDiskIndex);
/* Generate default external file names for external snapshot locations */
-if (virDomainSnapshotDe
[PATCH 04/13] qemuSnapshotCreateInactiveExternal: Don't access 'idx' of snapshot
After virDomainSnapshotAlignDisks is called the definitions of disks in
the snapshot definition and in the domain definition are in the same
order so they can be addressed using the same index.
This frees up 'idx' to be removed later.
Signed-off-by: Peter Krempa
---
src/qemu/qemu_snapshot.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/src/qemu/qemu_snapshot.c b/src/qemu/qemu_snapshot.c
index a6e091dd09..48a4e9dd41 100644
--- a/src/qemu/qemu_snapshot.c
+++ b/src/qemu/qemu_snapshot.c
@@ -171,7 +171,7 @@ qemuSnapshotCreateInactiveExternal(virQEMUDriverPtr driver,
* create them correctly. */
for (i = 0; i < snapdef->ndisks && !reuse; i++) {
snapdisk = &(snapdef->disks[i]);
-defdisk = snapdef->parent.dom->disks[snapdisk->idx];
+defdisk = vm->def->disks[i];
if (snapdisk->snapshot != VIR_DOMAIN_SNAPSHOT_LOCATION_EXTERNAL)
continue;
@@ -216,7 +216,7 @@ qemuSnapshotCreateInactiveExternal(virQEMUDriverPtr driver,
g_autoptr(virStorageSource) newsrc = NULL;
snapdisk = &(snapdef->disks[i]);
-defdisk = vm->def->disks[snapdisk->idx];
+defdisk = vm->def->disks[i];
if (snapdisk->snapshot != VIR_DOMAIN_SNAPSHOT_LOCATION_EXTERNAL)
continue;
--
2.26.2
[PATCH 00/13] virStorageSourceNew failure handling and virDomainSnapshotAlignDisks refactors
A set of patches that prevent failures from virStorageSourceNew and refactor virDomainSnapshotAlignDisks for better readability. Peter Krempa (13): virStorageSourceNew: Abort on failure virStorageVolDefParseXML: Use g_steal_pointer qemuDomainBlockRebase: Replace ternary operator with if/else qemuSnapshotCreateInactiveExternal: Don't access 'idx' of snapshot virDomainSnapshotAlignDisks: Refactor cleanup virDomainSnapshotAlignDisks: Rename 'def' -> 'snapdef' virDomainSnapshotAlignDisks: Rename 'disk' -> 'snapdisk' virDomainSnapshotAlignDisks: Add 'domdef' local variable virDomainSnapshotAlignDisks: Extract domain disk definition to a local variable virDomainSnapshotAlignDisks: remove unnecessary 'tmp' variable virDomainSnapshotAlignDisks: clarify handing of snapshot location virDomainSnapshotAlignDisks: refactor extension to all disks virDomainSnapshotDiskDef: Remove 'idx' field src/conf/backup_conf.c| 7 +- src/conf/domain_conf.c| 21 ++-- src/conf/snapshot_conf.c | 160 +++--- src/conf/snapshot_conf.h | 1 - src/conf/storage_conf.c | 8 +- src/qemu/qemu_domain.c| 21 ++-- src/qemu/qemu_driver.c| 18 ++- src/qemu/qemu_migration.c | 7 +- src/qemu/qemu_snapshot.c | 7 +- src/storage/storage_backend_gluster.c | 5 +- src/storage/storage_backend_logical.c | 4 +- src/storage/storage_util.c| 10 +- src/util/virstoragefile.c | 32 ++ tests/qemublocktest.c | 16 +-- tests/virstoragetest.c| 5 +- 15 files changed, 121 insertions(+), 201 deletions(-) -- 2.26.2
[PATCH 13/13] virDomainSnapshotDiskDef: Remove 'idx' field
It's no longer needed and is valid only after virDomainSnapshotAlignDisks
is called while holding the lock.
Signed-off-by: Peter Krempa
---
src/conf/snapshot_conf.h | 1 -
1 file changed, 1 deletion(-)
diff --git a/src/conf/snapshot_conf.h b/src/conf/snapshot_conf.h
index b5b1ef2718..fbc9b17c54 100644
--- a/src/conf/snapshot_conf.h
+++ b/src/conf/snapshot_conf.h
@@ -63,7 +63,6 @@ typedef struct _virDomainSnapshotDiskDef
virDomainSnapshotDiskDef;
typedef virDomainSnapshotDiskDef *virDomainSnapshotDiskDefPtr;
struct _virDomainSnapshotDiskDef {
char *name; /* name matching the dom->disks that matches name */
int snapshot; /* virDomainSnapshotLocation */
/* details of wrapper external file. src is always non-NULL.
--
2.26.2
[PATCH 01/13] virStorageSourceNew: Abort on failure
Add an abort() on the class/object allocation failures so that
virStorageSourceNew() always returns a virStorageSource and remove
checks from all callers.
Signed-off-by: Peter Krempa
---
src/conf/backup_conf.c| 7 ++
src/conf/domain_conf.c| 21 ++
src/conf/snapshot_conf.c | 7 ++
src/conf/storage_conf.c | 4 +---
src/qemu/qemu_domain.c| 21 ++
src/qemu/qemu_driver.c| 12 +++---
src/qemu/qemu_migration.c | 7 ++
src/qemu/qemu_snapshot.c | 3 +--
src/storage/storage_backend_gluster.c | 5 +
src/storage/storage_backend_logical.c | 4 +---
src/storage/storage_util.c| 10 +++--
src/util/virstoragefile.c | 32 ++-
tests/qemublocktest.c | 16 +++---
tests/virstoragetest.c| 5 +
14 files changed, 46 insertions(+), 108 deletions(-)
diff --git a/src/conf/backup_conf.c b/src/conf/backup_conf.c
index 5caba621d8..5c475239ad 100644
--- a/src/conf/backup_conf.c
+++ b/src/conf/backup_conf.c
@@ -169,8 +169,7 @@ virDomainBackupDiskDefParseXML(xmlNodePtr node,
def->state = tmp;
}
-if (!(def->store = virStorageSourceNew()))
-return -1;
+def->store = virStorageSourceNew();
if ((type = virXMLPropString(node, "type"))) {
if ((def->store->type = virStorageTypeFromString(type)) <= 0) {
@@ -500,9 +499,7 @@ virDomainBackupDefAssignStore(virDomainBackupDiskDefPtr
disk,
}
} else if (!disk->store) {
if (virStorageSourceGetActualType(src) == VIR_STORAGE_TYPE_FILE) {
-if (!(disk->store = virStorageSourceNew()))
-return -1;
-
+disk->store = virStorageSourceNew();
disk->store->type = VIR_STORAGE_TYPE_FILE;
disk->store->path = g_strdup_printf("%s.%s", src->path, suffix);
} else {
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index 9289c147fe..bbe3cddadf 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -2187,8 +2187,7 @@ virDomainDiskDefNew(virDomainXMLOptionPtr xmlopt)
if (VIR_ALLOC(ret) < 0)
return NULL;
-if (!(ret->src = virStorageSourceNew()))
-goto error;
+ret->src = virStorageSourceNew();
if (xmlopt &&
xmlopt->privateData.diskNew &&
@@ -2400,8 +2399,7 @@ virDomainFSDefNew(virDomainXMLOptionPtr xmlopt)
if (VIR_ALLOC(ret) < 0)
return NULL;
-if (!(ret->src = virStorageSourceNew()))
-goto cleanup;
+ret->src = virStorageSourceNew();
if (xmlopt &&
xmlopt->privateData.fsNew &&
@@ -8325,9 +8323,8 @@ virDomainHostdevSubsysSCSIHostDefParseXML(xmlNodePtr
sourcenode,
if (flags & VIR_DOMAIN_DEF_PARSE_STATUS &&
xmlopt && xmlopt->privateData.storageParse) {
if ((ctxt->node = virXPathNode("./privateData", ctxt))) {
-if (!scsihostsrc->src &&
-!(scsihostsrc->src = virStorageSourceNew()))
-return -1;
+if (!scsihostsrc->src)
+scsihostsrc->src = virStorageSourceNew();
if (xmlopt->privateData.storageParse(ctxt, scsihostsrc->src) < 0)
return -1;
}
@@ -8353,8 +8350,7 @@ virDomainHostdevSubsysSCSIiSCSIDefParseXML(xmlNodePtr
sourcenode,
/* For the purposes of command line creation, this needs to look
* like a disk storage source */
-if (!(iscsisrc->src = virStorageSourceNew()))
-return -1;
+iscsisrc->src = virStorageSourceNew();
iscsisrc->src->type = VIR_STORAGE_TYPE_NETWORK;
iscsisrc->src->protocol = VIR_STORAGE_NET_PROTOCOL_ISCSI;
@@ -9770,9 +9766,7 @@ virDomainStorageSourceParseBase(const char *type,
{
g_autoptr(virStorageSource) src = NULL;
-if (!(src = virStorageSourceNew()))
-return NULL;
-
+src = virStorageSourceNew();
src->type = VIR_STORAGE_TYPE_FILE;
if (type &&
@@ -9960,8 +9954,7 @@ virDomainDiskBackingStoreParse(xmlXPathContextPtr ctxt,
/* terminator does not have a type */
if (!(type = virXMLPropString(ctxt->node, "type"))) {
-if (!(src->backingStore = virStorageSourceNew()))
-return -1;
+src->backingStore = virStorageSourceNew();
return 0;
}
diff --git a/src/conf/snapshot_conf.c b/src/conf/snapshot_conf.c
index 1ee63b9141..87a00082ef 100644
--- a/src/conf/snapshot_conf.c
+++ b/src/conf/snapshot_conf.c
@@ -148,9 +148,7 @@ virDomainSnapshotDiskDefParseXML(xmlNodePtr node,
ctxt->node = node;
-if (!(def->src = virStorageSourceNew()))
-goto cleanup;
-
+def->src = virStorageSourceNew();
def->name = virXMLPropString(node, "name");
if (!def->name) {
virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
@@ -744,8 +742,7 @@ virDomainSnapshotAlignDisks(virDomainSnapshotDefPtr def,
if (virBitmapIsBitSet(map, i))
[PATCH 08/13] virDomainSnapshotAlignDisks: Add 'domdef' local variable
There are multiple places accessing the domain definition. Extract it to
a local variable so that it's more clear what's happening.
Signed-off-by: Peter Krempa
---
src/conf/snapshot_conf.c | 27 ++-
1 file changed, 14 insertions(+), 13 deletions(-)
diff --git a/src/conf/snapshot_conf.c b/src/conf/snapshot_conf.c
index c835ec7333..a1cee01944 100644
--- a/src/conf/snapshot_conf.c
+++ b/src/conf/snapshot_conf.c
@@ -649,33 +649,34 @@ virDomainSnapshotAlignDisks(virDomainSnapshotDefPtr
snapdef,
int default_snapshot,
bool require_match)
{
+virDomainDefPtr domdef = snapdef->parent.dom;
g_autoptr(virBitmap) map = NULL;
size_t i;
int ndisks;
-if (!snapdef->parent.dom) {
+if (!domdef) {
virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
_("missing domain in snapshot"));
return -1;
}
-if (snapdef->ndisks > snapdef->parent.dom->ndisks) {
+if (snapdef->ndisks > domdef->ndisks) {
virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
_("too many disk snapshot requests for domain"));
return -1;
}
/* Unlikely to have a guest without disks but technically possible. */
-if (!snapdef->parent.dom->ndisks)
+if (!domdef->ndisks)
return 0;
-if (!(map = virBitmapNew(snapdef->parent.dom->ndisks)))
+if (!(map = virBitmapNew(domdef->ndisks)))
return -1;
/* Double check requested disks. */
for (i = 0; i < snapdef->ndisks; i++) {
virDomainSnapshotDiskDefPtr snapdisk = &snapdef->disks[i];
-int idx = virDomainDiskIndexByName(snapdef->parent.dom,
snapdisk->name, false);
+int idx = virDomainDiskIndexByName(domdef, snapdisk->name, false);
int disk_snapshot;
if (idx < 0) {
@@ -693,7 +694,7 @@ virDomainSnapshotAlignDisks(virDomainSnapshotDefPtr snapdef,
ignore_value(virBitmapSetBit(map, idx));
snapdisk->idx = idx;
-disk_snapshot = snapdef->parent.dom->disks[idx]->snapshot;
+disk_snapshot = domdef->disks[idx]->snapshot;
if (!snapdisk->snapshot) {
if (disk_snapshot &&
(!require_match ||
@@ -721,33 +722,33 @@ virDomainSnapshotAlignDisks(virDomainSnapshotDefPtr
snapdef,
snapdisk->src->path, snapdisk->name);
return -1;
}
-if (STRNEQ(snapdisk->name, snapdef->parent.dom->disks[idx]->dst)) {
+if (STRNEQ(snapdisk->name, domdef->disks[idx]->dst)) {
VIR_FREE(snapdisk->name);
-snapdisk->name = g_strdup(snapdef->parent.dom->disks[idx]->dst);
+snapdisk->name = g_strdup(domdef->disks[idx]->dst);
}
}
/* Provide defaults for all remaining disks. */
ndisks = snapdef->ndisks;
if (VIR_EXPAND_N(snapdef->disks, snapdef->ndisks,
- snapdef->parent.dom->ndisks - snapdef->ndisks) < 0)
+ domdef->ndisks - snapdef->ndisks) < 0)
return -1;
-for (i = 0; i < snapdef->parent.dom->ndisks; i++) {
+for (i = 0; i < domdef->ndisks; i++) {
virDomainSnapshotDiskDefPtr snapdisk;
if (virBitmapIsBitSet(map, i))
continue;
snapdisk = &snapdef->disks[ndisks++];
snapdisk->src = virStorageSourceNew();
-snapdisk->name = g_strdup(snapdef->parent.dom->disks[i]->dst);
+snapdisk->name = g_strdup(domdef->disks[i]->dst);
snapdisk->idx = i;
/* Don't snapshot empty drives */
-if (virStorageSourceIsEmpty(snapdef->parent.dom->disks[i]->src))
+if (virStorageSourceIsEmpty(domdef->disks[i]->src))
snapdisk->snapshot = VIR_DOMAIN_SNAPSHOT_LOCATION_NONE;
else
-snapdisk->snapshot = snapdef->parent.dom->disks[i]->snapshot;
+snapdisk->snapshot = domdef->disks[i]->snapshot;
snapdisk->src->type = VIR_STORAGE_TYPE_FILE;
if (!snapdisk->snapshot)
--
2.26.2
[PATCH 05/13] virDomainSnapshotAlignDisks: Refactor cleanup
Use automatic pointer for the bitmap and get rid of the 'cleanup' label
and 'ret' variable.
Signed-off-by: Peter Krempa
---
src/conf/snapshot_conf.c | 33 +
1 file changed, 13 insertions(+), 20 deletions(-)
diff --git a/src/conf/snapshot_conf.c b/src/conf/snapshot_conf.c
index 87a00082ef..160f2054a4 100644
--- a/src/conf/snapshot_conf.c
+++ b/src/conf/snapshot_conf.c
@@ -649,31 +649,28 @@ virDomainSnapshotAlignDisks(virDomainSnapshotDefPtr def,
int default_snapshot,
bool require_match)
{
-int ret = -1;
-virBitmapPtr map = NULL;
+g_autoptr(virBitmap) map = NULL;
size_t i;
int ndisks;
if (!def->parent.dom) {
virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
_("missing domain in snapshot"));
-goto cleanup;
+return -1;
}
if (def->ndisks > def->parent.dom->ndisks) {
virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
_("too many disk snapshot requests for domain"));
-goto cleanup;
+return -1;
}
/* Unlikely to have a guest without disks but technically possible. */
-if (!def->parent.dom->ndisks) {
-ret = 0;
-goto cleanup;
-}
+if (!def->parent.dom->ndisks)
+return 0;
if (!(map = virBitmapNew(def->parent.dom->ndisks)))
-goto cleanup;
+return -1;
/* Double check requested disks. */
for (i = 0; i < def->ndisks; i++) {
@@ -684,14 +681,14 @@ virDomainSnapshotAlignDisks(virDomainSnapshotDefPtr def,
if (idx < 0) {
virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
_("no disk named '%s'"), disk->name);
-goto cleanup;
+return -1;
}
if (virBitmapIsBitSet(map, idx)) {
virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
_("disk '%s' specified twice"),
disk->name);
-goto cleanup;
+return -1;
}
ignore_value(virBitmapSetBit(map, idx));
disk->idx = idx;
@@ -714,7 +711,7 @@ virDomainSnapshotAlignDisks(virDomainSnapshotDefPtr def,
virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
_("disk '%s' must use snapshot mode '%s'"),
disk->name, tmp);
-goto cleanup;
+return -1;
}
if (disk->src->path &&
disk->snapshot != VIR_DOMAIN_SNAPSHOT_LOCATION_EXTERNAL) {
@@ -722,7 +719,7 @@ virDomainSnapshotAlignDisks(virDomainSnapshotDefPtr def,
_("file '%s' for disk '%s' requires "
"use of external snapshot mode"),
disk->src->path, disk->name);
-goto cleanup;
+return -1;
}
if (STRNEQ(disk->name, def->parent.dom->disks[idx]->dst)) {
VIR_FREE(disk->name);
@@ -734,7 +731,7 @@ virDomainSnapshotAlignDisks(virDomainSnapshotDefPtr def,
ndisks = def->ndisks;
if (VIR_EXPAND_N(def->disks, def->ndisks,
def->parent.dom->ndisks - def->ndisks) < 0)
-goto cleanup;
+return -1;
for (i = 0; i < def->parent.dom->ndisks; i++) {
virDomainSnapshotDiskDefPtr disk;
@@ -762,13 +759,9 @@ virDomainSnapshotAlignDisks(virDomainSnapshotDefPtr def,
/* Generate default external file names for external snapshot locations */
if (virDomainSnapshotDefAssignExternalNames(def) < 0)
-goto cleanup;
-
-ret = 0;
+return -1;
- cleanup:
-virBitmapFree(map);
-return ret;
+return 0;
}
--
2.26.2
[PATCH 02/13] virStorageVolDefParseXML: Use g_steal_pointer
Signed-off-by: Peter Krempa
---
src/conf/storage_conf.c | 4 +---
1 file changed, 1 insertion(+), 3 deletions(-)
diff --git a/src/conf/storage_conf.c b/src/conf/storage_conf.c
index 2b00f09d73..d53d50479b 100644
--- a/src/conf/storage_conf.c
+++ b/src/conf/storage_conf.c
@@ -1352,9 +1352,7 @@ virStorageVolDefParseXML(virStoragePoolDefPtr pool,
if ((backingStore = virXPathString("string(./backingStore/path)", ctxt))) {
def->target.backingStore = virStorageSourceNew();
def->target.backingStore->type = VIR_STORAGE_TYPE_FILE;
-
-def->target.backingStore->path = backingStore;
-backingStore = NULL;
+def->target.backingStore->path = g_steal_pointer(&backingStore);
if (options->formatFromString) {
g_autofree char *format = NULL;
--
2.26.2
[PATCH 11/13] virDomainSnapshotAlignDisks: clarify handing of snapshot location
Remove the use of the 'disk_snapshot' temporary variable since accessing
the disk definition now isn't that much longer to write and use expicit
value checks instead of the (non-)zero check to make it more obvious
what the code is doing.
Signed-off-by: Peter Krempa
---
src/conf/snapshot_conf.c | 16
1 file changed, 8 insertions(+), 8 deletions(-)
diff --git a/src/conf/snapshot_conf.c b/src/conf/snapshot_conf.c
index 10eb584a1c..f6a827d2ff 100644
--- a/src/conf/snapshot_conf.c
+++ b/src/conf/snapshot_conf.c
@@ -678,7 +678,6 @@ virDomainSnapshotAlignDisks(virDomainSnapshotDefPtr snapdef,
virDomainSnapshotDiskDefPtr snapdisk = &snapdef->disks[i];
int idx = virDomainDiskIndexByName(domdef, snapdisk->name, false);
virDomainDiskDefPtr domdisk = NULL;
-int disk_snapshot;
if (idx < 0) {
virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
@@ -697,24 +696,25 @@ virDomainSnapshotAlignDisks(virDomainSnapshotDefPtr
snapdef,
ignore_value(virBitmapSetBit(map, idx));
snapdisk->idx = idx;
-disk_snapshot = domdisk->snapshot;
-if (!snapdisk->snapshot) {
-if (disk_snapshot &&
+if (snapdisk->snapshot == VIR_DOMAIN_SNAPSHOT_LOCATION_DEFAULT) {
+if (domdisk->snapshot != VIR_DOMAIN_SNAPSHOT_LOCATION_DEFAULT &&
(!require_match ||
- disk_snapshot == VIR_DOMAIN_SNAPSHOT_LOCATION_NONE))
-snapdisk->snapshot = disk_snapshot;
-else
+ domdisk->snapshot == VIR_DOMAIN_SNAPSHOT_LOCATION_NONE)) {
+snapdisk->snapshot = domdisk->snapshot;
+} else {
snapdisk->snapshot = default_snapshot;
+}
} else if (require_match &&
snapdisk->snapshot != default_snapshot &&
!(snapdisk->snapshot == VIR_DOMAIN_SNAPSHOT_LOCATION_NONE &&
- disk_snapshot == VIR_DOMAIN_SNAPSHOT_LOCATION_NONE)) {
+ domdisk->snapshot == VIR_DOMAIN_SNAPSHOT_LOCATION_NONE)) {
virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
_("disk '%s' must use snapshot mode '%s'"),
snapdisk->name,
virDomainSnapshotLocationTypeToString(default_snapshot));
return -1;
}
+
if (snapdisk->src->path &&
snapdisk->snapshot != VIR_DOMAIN_SNAPSHOT_LOCATION_EXTERNAL) {
virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
--
2.26.2
[PATCH 12/13] virDomainSnapshotAlignDisks: refactor extension to all disks
Last step of the algorithm in virDomainSnapshotAlignDisks is to extend
the array of disks to all VM's disk and provide defaults. This was done
by extending the array, adding defaults at the end and then sorting it.
This requires the 'idx' variable and also a separate sorting function.
If we store the pointer to existing snapshot disk definitions in a hash
table and create a new array of snapshot disk definitions, we can fill
the new array directly by either copying the definition from the old
array or adding the default.
This avoids the sorting step and thus even the need to store the index
of the domain disk altogether.
Signed-off-by: Peter Krempa
---
src/conf/snapshot_conf.c | 48 +++-
1 file changed, 18 insertions(+), 30 deletions(-)
diff --git a/src/conf/snapshot_conf.c b/src/conf/snapshot_conf.c
index f6a827d2ff..7090e3a1b0 100644
--- a/src/conf/snapshot_conf.c
+++ b/src/conf/snapshot_conf.c
@@ -627,16 +627,6 @@
virDomainSnapshotDefAssignExternalNames(virDomainSnapshotDefPtr def)
}
-static int
-virDomainSnapshotCompareDiskIndex(const void *a, const void *b)
-{
-const virDomainSnapshotDiskDef *diska = a;
-const virDomainSnapshotDiskDef *diskb = b;
-
-/* Integer overflow shouldn't be a problem here. */
-return diska->idx - diskb->idx;
-}
-
/* Align def->disks to def->parent.dom. Sort the list of def->disks,
* filling in any missing disks or snapshot state defaults given by
* the domain, with a fallback to a passed in default. Convert paths
@@ -650,9 +640,9 @@ virDomainSnapshotAlignDisks(virDomainSnapshotDefPtr snapdef,
bool require_match)
{
virDomainDefPtr domdef = snapdef->parent.dom;
-g_autoptr(virBitmap) map = NULL;
+g_autoptr(virHashTable) map = virHashNew(NULL);
+g_autofree virDomainSnapshotDiskDefPtr olddisks = NULL;
size_t i;
-int ndisks;
if (!domdef) {
virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
@@ -670,9 +660,6 @@ virDomainSnapshotAlignDisks(virDomainSnapshotDefPtr snapdef,
if (!domdef->ndisks)
return 0;
-if (!(map = virBitmapNew(domdef->ndisks)))
-return -1;
-
/* Double check requested disks. */
for (i = 0; i < snapdef->ndisks; i++) {
virDomainSnapshotDiskDefPtr snapdisk = &snapdef->disks[i];
@@ -687,14 +674,15 @@ virDomainSnapshotAlignDisks(virDomainSnapshotDefPtr
snapdef,
domdisk = domdef->disks[idx];
-if (virBitmapIsBitSet(map, idx)) {
+if (virHashHasEntry(map, domdisk->dst)) {
virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
_("disk '%s' specified twice"),
snapdisk->name);
return -1;
}
-ignore_value(virBitmapSetBit(map, idx));
-snapdisk->idx = idx;
+
+if (virHashAddEntry(map, domdisk->dst, snapdisk) < 0)
+return -1;
if (snapdisk->snapshot == VIR_DOMAIN_SNAPSHOT_LOCATION_DEFAULT) {
if (domdisk->snapshot != VIR_DOMAIN_SNAPSHOT_LOCATION_DEFAULT &&
@@ -729,21 +717,24 @@ virDomainSnapshotAlignDisks(virDomainSnapshotDefPtr
snapdef,
}
}
-/* Provide defaults for all remaining disks. */
-ndisks = snapdef->ndisks;
-if (VIR_EXPAND_N(snapdef->disks, snapdef->ndisks,
- domdef->ndisks - snapdef->ndisks) < 0)
-return -1;
+olddisks = g_steal_pointer(&snapdef->disks);
+snapdef->disks = g_new0(virDomainSnapshotDiskDef, domdef->ndisks);
+snapdef->ndisks = domdef->ndisks;
for (i = 0; i < domdef->ndisks; i++) {
-virDomainSnapshotDiskDefPtr snapdisk;
+virDomainDiskDefPtr domdisk = domdef->disks[i];
+virDomainSnapshotDiskDefPtr snapdisk = snapdef->disks + i;
+virDomainSnapshotDiskDefPtr existing;
-if (virBitmapIsBitSet(map, i))
+/* copy existing disks */
+if ((existing = virHashLookup(map, domdisk->dst))) {
+memcpy(snapdisk, existing, sizeof(*snapdisk));
continue;
-snapdisk = &snapdef->disks[ndisks++];
+}
+
+/* Provide defaults for all remaining disks. */
snapdisk->src = virStorageSourceNew();
snapdisk->name = g_strdup(domdef->disks[i]->dst);
-snapdisk->idx = i;
/* Don't snapshot empty drives */
if (virStorageSourceIsEmpty(domdef->disks[i]->src))
@@ -756,9 +747,6 @@ virDomainSnapshotAlignDisks(virDomainSnapshotDefPtr snapdef,
snapdisk->snapshot = default_snapshot;
}
-qsort(&snapdef->disks[0], snapdef->ndisks, sizeof(snapdef->disks[0]),
- virDomainSnapshotCompareDiskIndex);
-
/* Generate default external file names for external snapshot locations */
if (virDomainSnapshotDefAssignExternalNames(snapdef) < 0)
return -1;
--
2.26.2
[PATCH 07/13] virDomainSnapshotAlignDisks: Rename 'disk' -> 'snapdisk'
The 'disk' variable usually refers to a definition of a disk from the
domain definition. Rename it to 'snapdisk' to be clear that we are
talking about the snapshot disk definition especially since this
function also accesses the domain disk definition.
Signed-off-by: Peter Krempa
---
src/conf/snapshot_conf.c | 54
1 file changed, 27 insertions(+), 27 deletions(-)
diff --git a/src/conf/snapshot_conf.c b/src/conf/snapshot_conf.c
index 77f53c4a1d..c835ec7333 100644
--- a/src/conf/snapshot_conf.c
+++ b/src/conf/snapshot_conf.c
@@ -674,56 +674,56 @@ virDomainSnapshotAlignDisks(virDomainSnapshotDefPtr
snapdef,
/* Double check requested disks. */
for (i = 0; i < snapdef->ndisks; i++) {
-virDomainSnapshotDiskDefPtr disk = &snapdef->disks[i];
-int idx = virDomainDiskIndexByName(snapdef->parent.dom, disk->name,
false);
+virDomainSnapshotDiskDefPtr snapdisk = &snapdef->disks[i];
+int idx = virDomainDiskIndexByName(snapdef->parent.dom,
snapdisk->name, false);
int disk_snapshot;
if (idx < 0) {
virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
- _("no disk named '%s'"), disk->name);
+ _("no disk named '%s'"), snapdisk->name);
return -1;
}
if (virBitmapIsBitSet(map, idx)) {
virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
_("disk '%s' specified twice"),
- disk->name);
+ snapdisk->name);
return -1;
}
ignore_value(virBitmapSetBit(map, idx));
-disk->idx = idx;
+snapdisk->idx = idx;
disk_snapshot = snapdef->parent.dom->disks[idx]->snapshot;
-if (!disk->snapshot) {
+if (!snapdisk->snapshot) {
if (disk_snapshot &&
(!require_match ||
disk_snapshot == VIR_DOMAIN_SNAPSHOT_LOCATION_NONE))
-disk->snapshot = disk_snapshot;
+snapdisk->snapshot = disk_snapshot;
else
-disk->snapshot = default_snapshot;
+snapdisk->snapshot = default_snapshot;
} else if (require_match &&
- disk->snapshot != default_snapshot &&
- !(disk->snapshot == VIR_DOMAIN_SNAPSHOT_LOCATION_NONE &&
+ snapdisk->snapshot != default_snapshot &&
+ !(snapdisk->snapshot == VIR_DOMAIN_SNAPSHOT_LOCATION_NONE &&
disk_snapshot == VIR_DOMAIN_SNAPSHOT_LOCATION_NONE)) {
const char *tmp;
tmp = virDomainSnapshotLocationTypeToString(default_snapshot);
virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
_("disk '%s' must use snapshot mode '%s'"),
- disk->name, tmp);
+ snapdisk->name, tmp);
return -1;
}
-if (disk->src->path &&
-disk->snapshot != VIR_DOMAIN_SNAPSHOT_LOCATION_EXTERNAL) {
+if (snapdisk->src->path &&
+snapdisk->snapshot != VIR_DOMAIN_SNAPSHOT_LOCATION_EXTERNAL) {
virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
_("file '%s' for disk '%s' requires "
"use of external snapshot mode"),
- disk->src->path, disk->name);
+ snapdisk->src->path, snapdisk->name);
return -1;
}
-if (STRNEQ(disk->name, snapdef->parent.dom->disks[idx]->dst)) {
-VIR_FREE(disk->name);
-disk->name = g_strdup(snapdef->parent.dom->disks[idx]->dst);
+if (STRNEQ(snapdisk->name, snapdef->parent.dom->disks[idx]->dst)) {
+VIR_FREE(snapdisk->name);
+snapdisk->name = g_strdup(snapdef->parent.dom->disks[idx]->dst);
}
}
@@ -734,24 +734,24 @@ virDomainSnapshotAlignDisks(virDomainSnapshotDefPtr
snapdef,
return -1;
for (i = 0; i < snapdef->parent.dom->ndisks; i++) {
-virDomainSnapshotDiskDefPtr disk;
+virDomainSnapshotDiskDefPtr snapdisk;
if (virBitmapIsBitSet(map, i))
continue;
-disk = &snapdef->disks[ndisks++];
-disk->src = virStorageSourceNew();
-disk->name = g_strdup(snapdef->parent.dom->disks[i]->dst);
-disk->idx = i;
+snapdisk = &snapdef->disks[ndisks++];
+snapdisk->src = virStorageSourceNew();
+snapdisk->name = g_strdup(snapdef->parent.dom->disks[i]->dst);
+snapdisk->idx = i;
/* Don't snapshot empty drives */
if (virStorageSourceIsEmpty(snapdef->parent.dom->disks[i]->src))
-disk->snapshot = VIR_DOMAIN_SNAPSHOT_LOCATION_NONE;
+snapdisk->snapshot = VIR_DOMAIN_SNAPSHOT_LOCATION_NONE;
else
-disk->snapshot = snapdef->parent.dom->disks[i]->snapshot;
+snapdisk->snapsho
[PATCH 10/13] virDomainSnapshotAlignDisks: remove unnecessary 'tmp' variable
The converted string is used exactly once so we can call the conversion
without storing the result in a variable.
Signed-off-by: Peter Krempa
---
src/conf/snapshot_conf.c | 6 ++
1 file changed, 2 insertions(+), 4 deletions(-)
diff --git a/src/conf/snapshot_conf.c b/src/conf/snapshot_conf.c
index aeebe2fb33..10eb584a1c 100644
--- a/src/conf/snapshot_conf.c
+++ b/src/conf/snapshot_conf.c
@@ -709,12 +709,10 @@ virDomainSnapshotAlignDisks(virDomainSnapshotDefPtr
snapdef,
snapdisk->snapshot != default_snapshot &&
!(snapdisk->snapshot == VIR_DOMAIN_SNAPSHOT_LOCATION_NONE &&
disk_snapshot == VIR_DOMAIN_SNAPSHOT_LOCATION_NONE)) {
-const char *tmp;
-
-tmp = virDomainSnapshotLocationTypeToString(default_snapshot);
virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
_("disk '%s' must use snapshot mode '%s'"),
- snapdisk->name, tmp);
+ snapdisk->name,
+
virDomainSnapshotLocationTypeToString(default_snapshot));
return -1;
}
if (snapdisk->src->path &&
--
2.26.2
[PATCH 03/13] qemuDomainBlockRebase: Replace ternary operator with if/else
Signed-off-by: Peter Krempa --- src/qemu/qemu_driver.c | 6 -- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 9b5ff3a65c..40d11aced6 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -15292,8 +15292,10 @@ qemuDomainBlockRebase(virDomainPtr dom, const char *path, const char *base, /* If we got here, we are doing a block copy rebase. */ dest = virStorageSourceNew(); -dest->type = (flags & VIR_DOMAIN_BLOCK_REBASE_COPY_DEV) ? -VIR_STORAGE_TYPE_BLOCK : VIR_STORAGE_TYPE_FILE; +if (flags & VIR_DOMAIN_BLOCK_REBASE_COPY_DEV) +dest->type = VIR_STORAGE_TYPE_BLOCK; +else +dest->type = VIR_STORAGE_TYPE_FILE; dest->path = g_strdup(base); if (flags & VIR_DOMAIN_BLOCK_REBASE_COPY_RAW) dest->format = VIR_STORAGE_FILE_RAW; -- 2.26.2
[PATCH 09/13] virDomainSnapshotAlignDisks: Extract domain disk definition to a local variable
Extract the disk def to a local variable so that it's more obvious
what's happening and it will also allow further simplification.
Signed-off-by: Peter Krempa
---
src/conf/snapshot_conf.c | 9 ++---
1 file changed, 6 insertions(+), 3 deletions(-)
diff --git a/src/conf/snapshot_conf.c b/src/conf/snapshot_conf.c
index a1cee01944..aeebe2fb33 100644
--- a/src/conf/snapshot_conf.c
+++ b/src/conf/snapshot_conf.c
@@ -677,6 +677,7 @@ virDomainSnapshotAlignDisks(virDomainSnapshotDefPtr snapdef,
for (i = 0; i < snapdef->ndisks; i++) {
virDomainSnapshotDiskDefPtr snapdisk = &snapdef->disks[i];
int idx = virDomainDiskIndexByName(domdef, snapdisk->name, false);
+virDomainDiskDefPtr domdisk = NULL;
int disk_snapshot;
if (idx < 0) {
@@ -685,6 +686,8 @@ virDomainSnapshotAlignDisks(virDomainSnapshotDefPtr snapdef,
return -1;
}
+domdisk = domdef->disks[idx];
+
if (virBitmapIsBitSet(map, idx)) {
virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
_("disk '%s' specified twice"),
@@ -694,7 +697,7 @@ virDomainSnapshotAlignDisks(virDomainSnapshotDefPtr snapdef,
ignore_value(virBitmapSetBit(map, idx));
snapdisk->idx = idx;
-disk_snapshot = domdef->disks[idx]->snapshot;
+disk_snapshot = domdisk->snapshot;
if (!snapdisk->snapshot) {
if (disk_snapshot &&
(!require_match ||
@@ -722,9 +725,9 @@ virDomainSnapshotAlignDisks(virDomainSnapshotDefPtr snapdef,
snapdisk->src->path, snapdisk->name);
return -1;
}
-if (STRNEQ(snapdisk->name, domdef->disks[idx]->dst)) {
+if (STRNEQ(snapdisk->name, domdisk->dst)) {
VIR_FREE(snapdisk->name);
-snapdisk->name = g_strdup(domdef->disks[idx]->dst);
+snapdisk->name = g_strdup(domdisk->dst);
}
}
--
2.26.2
[libvirt PATCHv2 2/2] tests: use g_new0 instead of VIR_ALLOC_N
Signed-off-by: Ján Tomko
---
tests/commandtest.c | 7 +++
tests/domaincapstest.c | 3 +--
tests/fdstreamtest.c | 10 --
tests/qemuxml2argvtest.c | 3 +--
tests/securityselinuxlabeltest.c | 3 +--
tests/securityselinuxtest.c | 3 +--
tests/testutils.c| 8 ++--
tests/testutilsqemu.c| 6 ++
tests/vircgrouptest.c| 3 +--
tests/vircryptotest.c| 5 ++---
tests/virhostcputest.c | 3 +--
tests/virnetmessagetest.c| 6 ++
tests/virnetsockettest.c | 6 +-
tests/virnettlshelpers.c | 3 +--
tests/virstringtest.c| 3 +--
tests/xlconfigtest.c | 3 +--
tests/xmconfigtest.c | 3 +--
17 files changed, 26 insertions(+), 52 deletions(-)
diff --git a/tests/commandtest.c b/tests/commandtest.c
index cbbcda4e5f..df86725f0e 100644
--- a/tests/commandtest.c
+++ b/tests/commandtest.c
@@ -1057,10 +1057,9 @@ static int test27(const void *unused G_GNUC_UNUSED)
"%s%s%s" \
"END STDERR\n"
-if (VIR_ALLOC_N(buffer0, buflen) < 0 ||
-VIR_ALLOC_N(buffer1, buflen) < 0 ||
-VIR_ALLOC_N(buffer2, buflen) < 0)
-goto cleanup;
+buffer0 = g_new0(char, buflen);
+buffer1 = g_new0(char, buflen);
+buffer2 = g_new0(char, buflen);
memset(buffer0, 'H', buflen - 2);
buffer0[buflen - 2] = '\n';
diff --git a/tests/domaincapstest.c b/tests/domaincapstest.c
index f817ed5452..7a082705c6 100644
--- a/tests/domaincapstest.c
+++ b/tests/domaincapstest.c
@@ -147,8 +147,7 @@ fillXenCaps(virDomainCapsPtr domCaps)
virFirmwarePtr *firmwares;
int ret = -1;
-if (VIR_ALLOC_N(firmwares, 2) < 0)
-return ret;
+firmwares = g_new0(virFirmwarePtr, 2);
firmwares[0] = g_new0(virFirmware, 1);
firmwares[1] = g_new0(virFirmware, 1);
diff --git a/tests/fdstreamtest.c b/tests/fdstreamtest.c
index 83973137e7..7a2fe27477 100644
--- a/tests/fdstreamtest.c
+++ b/tests/fdstreamtest.c
@@ -54,9 +54,8 @@ static int testFDStreamReadCommon(const char *scratchdir,
bool blocking)
if (!(conn = virConnectOpen("test:///default")))
goto cleanup;
-if (VIR_ALLOC_N(pattern, PATTERN_LEN) < 0 ||
-VIR_ALLOC_N(buf, PATTERN_LEN) < 0)
-goto cleanup;
+pattern = g_new0(char, PATTERN_LEN);
+buf = g_new0(char, PATTERN_LEN);
for (i = 0; i < PATTERN_LEN; i++)
pattern[i] = i;
@@ -185,9 +184,8 @@ static int testFDStreamWriteCommon(const char *scratchdir,
bool blocking)
if (!(conn = virConnectOpen("test:///default")))
goto cleanup;
-if (VIR_ALLOC_N(pattern, PATTERN_LEN) < 0 ||
-VIR_ALLOC_N(buf, PATTERN_LEN) < 0)
-goto cleanup;
+pattern = g_new0(char, PATTERN_LEN);
+buf = g_new0(char, PATTERN_LEN);
for (i = 0; i < PATTERN_LEN; i++)
pattern[i] = i;
diff --git a/tests/qemuxml2argvtest.c b/tests/qemuxml2argvtest.c
index e93948e3fc..463e4c003d 100644
--- a/tests/qemuxml2argvtest.c
+++ b/tests/qemuxml2argvtest.c
@@ -753,8 +753,7 @@ mymain(void)
driver.config->nbdTLSx509certdir =
g_strdup("/etc/pki/libvirt-nbd/dummy,path");
VIR_FREE(driver.config->hugetlbfs);
-if (VIR_ALLOC_N(driver.config->hugetlbfs, 2) < 0)
-return EXIT_FAILURE;
+driver.config->hugetlbfs = g_new0(virHugeTLBFS, 2);
driver.config->nhugetlbfs = 2;
driver.config->hugetlbfs[0].mnt_dir = g_strdup("/dev/hugepages2M");
driver.config->hugetlbfs[1].mnt_dir = g_strdup("/dev/hugepages1G");
diff --git a/tests/securityselinuxlabeltest.c b/tests/securityselinuxlabeltest.c
index 2989a668b7..168acc2bdf 100644
--- a/tests/securityselinuxlabeltest.c
+++ b/tests/securityselinuxlabeltest.c
@@ -114,8 +114,7 @@ testSELinuxLoadFileList(const char *testname,
if (!(fp = fopen(path, "r")))
goto cleanup;
-if (VIR_ALLOC_N(line, 1024) < 0)
-goto cleanup;
+line = g_new0(char, 1024);
while (!feof(fp)) {
char *file = NULL, *context = NULL, *tmp;
diff --git a/tests/securityselinuxtest.c b/tests/securityselinuxtest.c
index 297cc0e53d..a75ff495eb 100644
--- a/tests/securityselinuxtest.c
+++ b/tests/securityselinuxtest.c
@@ -71,8 +71,7 @@ testBuildDomainDef(bool dynamic,
goto error;
def->virtType = VIR_DOMAIN_VIRT_KVM;
-if (VIR_ALLOC_N(def->seclabels, 1) < 0)
-goto error;
+def->seclabels = g_new0(virSecurityLabelDefPtr, 1);
secdef = g_new0(virSecurityLabelDef, 1);
diff --git a/tests/testutils.c b/tests/testutils.c
index 3f53f635fc..b747f65eea 100644
--- a/tests/testutils.c
+++ b/tests/testutils.c
@@ -211,10 +211,7 @@ virTestLoadFile(const char *file, char **buf)
tmplen = buflen = st.st_size + 1;
-if (VIR_ALLOC_N(*buf, buflen) < 0) {
-VIR_FORCE_FCLOSE(fp);
-return -1;
-}
+*buf = g_new0(char, buflen);
tmp = *buf;
(*buf)[0] = '\0';
@@ -977,8 +974,7 @@ virTestCapsBuildNUMATopolog
[libvirt PATCHv2 0/2] tests: switch to g_new0 (glib chronicles)
This time actually compile-tested. Ján Tomko (2): tests: build SELinux tests tests: use g_new0 instead of VIR_ALLOC_N tests/commandtest.c | 7 +++ tests/domaincapstest.c | 3 +-- tests/fdstreamtest.c | 10 -- tests/meson.build| 2 +- tests/qemuxml2argvtest.c | 3 +-- tests/securityselinuxlabeltest.c | 3 +-- tests/securityselinuxtest.c | 3 +-- tests/testutils.c| 8 ++-- tests/testutilsqemu.c| 6 ++ tests/vircgrouptest.c| 3 +-- tests/vircryptotest.c| 5 ++--- tests/virhostcputest.c | 3 +-- tests/virnetmessagetest.c| 6 ++ tests/virnetsockettest.c | 6 +- tests/virnettlshelpers.c | 3 +-- tests/virstringtest.c| 3 +-- tests/xlconfigtest.c | 3 +-- tests/xmconfigtest.c | 3 +-- 18 files changed, 27 insertions(+), 53 deletions(-) -- 2.26.2
[libvirt PATCHv2 1/2] tests: build SELinux tests
We set WITH_LIBATTR in meson.build, not WITH_ATTR.
Signed-off-by: Ján Tomko
Fixes: 3ace72965c3b11fc763f781ae7ce3ca29dd36507
---
tests/meson.build | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/tests/meson.build b/tests/meson.build
index 31e8d66c7a..eaf6628fb9 100644
--- a/tests/meson.build
+++ b/tests/meson.build
@@ -477,7 +477,7 @@ if conf.has('WITH_REMOTE')
endif
if conf.has('WITH_SECDRIVER_SELINUX')
- if conf.has('WITH_ATTR')
+ if conf.has('WITH_LIBATTR')
tests += [
{ 'name': 'securityselinuxtest' },
{ 'name': 'viridentitytest' },
--
2.26.2
Re: [PATCH v1] qemu: monitor: substitute missing model name for host-passthrough
Collin, I apologize for not getting to you earlier. On Wed, Sep 16, 2020 at 12:11:08 -0400, Collin Walling wrote: > On 9/16/20 3:03 AM, Michal Privoznik wrote: > > On 9/15/20 10:25 PM, Collin Walling wrote: > >> One more ping in attempt to get this in the right direction. Otherwise > >> I'll post my next idea and we can go from there :) > > > > I agree with Peter that while the idea might look correct it's too deep. > > > >> > >> Thinking about this issue, should a host-passthough CPU definition be > >> permitted for the baseline & comparison commands? The model represented > >> under this mode is not considered migration safe and it may make sense > >> to simply fail early since these commands aim to construct/determine a > >> migratable CPU model, respectively. > > > > Honestly, I don't know much about this CPU models area, but is that true > > even for two identical hosts? Say I have two desktops next to each > > other, with the same CPU and I want to migrate. I could use host model, > > couldn't I? > > > > "Host-model" is an alias for a CPU model that closely represents the > capabilities of the host machine (on s390, because this model is defined > by the hypervisor, it can also be called the "hypervisor CPU model" -- > not an important detail). > > However, a guest running with the host-passthrough mode is not > considered migration safe as that guest may covertly run with > features/capabilities that are not directly exposed to the hypervisor. Right, but migration may still be possible and working fine if both host are identical. > From what I understand regarding the hypervisor-cpu-compare and > hypervisor-cpu-baseline commands is that they aim to assist with > determining the migratability of guests based on their CPU model and > feature set (usually along with a host CPU in the equation as well). Baseline with a host-passthrough CPU is not indeed very useful, but compare could still be used and its usage is not limited to migration. For example, you can use it to check whether a domain with a guest CPU configuration can be started on a specific host before you actually try to start it. And reporting host-passthrough as incompatible would be wrong. Anyway, thanks for your patch, it was mostly correct, it just needed to be done a bit higher in the call graph. Incidentally, Tim Wiederhake [1] took this original patch and moved the change to the right place. The authorship is still yours, so if you want to append you signed-off-by tag there, I'll wait a bit before pushing Tim's patch. Jirka [1] https://www.redhat.com/archives/libvir-list/2020-September/msg01177.html
Re: [PATCH] apparmor: Allow /usr/libexec for libxl-save-helper and pygrub
On 9/23/20 7:26 AM, Christian Ehrhardt wrote: On Wed, Sep 23, 2020 at 12:35 AM Jim Fehlig wrote: Like other distros, openSUSE Tumbleweed recently changed libexecdir from /usr/lib to /usr/libexec. Add it as an allowed path for libxl-save-helper and pygrub. Hi Jim, ack to the intention, but I think since this should use @libexecdir@ I think. Or did anything change that this doesn't apply anymore ... in that case I beg your pardon. [1]: https://libvirt.org/git/?p=libvirt.git;a=commit;h=5c8bd31c881e99261ac098e867a79b300440731a Heh, I see that skipped over the xen stuff :-). I'll send a V2 later. Regards, Jim
Re: [PATCH v2 2/2] block: deprecate the sheepdog block driver
Daniel P. Berrangé writes:
> This thread from a little over a year ago:
>
> http://lists.wpkg.org/pipermail/sheepdog/2019-March/thread.html
>
> states that sheepdog is no longer actively developed. The only mentioned
> users are some companies who are said to have it for legacy reasons with
> plans to replace it by Ceph. There is talk about cutting out existing
> features to turn it into a simple demo of how to write a distributed
> block service. There is no evidence of anyone working on that idea:
>
> https://github.com/sheepdog/sheepdog/commits/master
>
> No real commits to git since Jan 2018, and before then just some minor
> technical debt cleanup..
Drop the extra period.
>
> There is essentially no activity on the mailing list aside from
> patches to QEMU that get CC'd due to our MAINTAINERS entry.
>
> Fedora packages for sheepdog failed to build from upstream source
> because of the more strict linker that no longer merges duplicate
> global symbols. Fedora patches it to add the missing "extern"
> annotations and presumably other distros do to, but upstream source
> remains broken.
>
> There is only basic compile testing, no functional testing of the
> driver.
>
> Since there are no build pre-requisites the sheepdog driver is currently
> enabled unconditionally. This would result in configure issuing a
> deprecation warning by default for all users. Thus the configure default
> is changed to disable it, requiring users to pass --enable-sheepdog to
> build the driver.
>
> Signed-off-by: Daniel P. Berrangé
> ---
> block/sheepdog.c | 15 +++
> configure | 5 +++--
> docs/system/deprecated.rst | 9 +
> 3 files changed, 27 insertions(+), 2 deletions(-)
>
> diff --git a/block/sheepdog.c b/block/sheepdog.c
> index cbbebc1aaf..7f68bd6a1a 100644
> --- a/block/sheepdog.c
> +++ b/block/sheepdog.c
> @@ -242,6 +242,17 @@ typedef struct SheepdogInode {
> */
> #define FNV1A_64_INIT ((uint64_t)0xcbf29ce484222325ULL)
>
> +static void deprecation_warning(void)
> +{
> +static bool warned = false;
Obey checkpatch :)
> +
> +if (!warned) {
> +warn_report("the sheepdog block driver is deprecated and will be "
> +"removed in a future release");
Similar warnings elsewhere don't say "will be removed".
Some of them are nice enough to advise what to use instead, but that may
not be practical here.
> +warned = true;
> +}
> +}
> +
> /*
> * 64 bit Fowler/Noll/Vo FNV-1a hash code
> */
> @@ -1548,6 +1559,8 @@ static int sd_open(BlockDriverState *bs, QDict
> *options, int flags,
> char *buf = NULL;
> QemuOpts *opts;
>
> +deprecation_warning();
> +
> s->bs = bs;
> s->aio_context = bdrv_get_aio_context(bs);
>
> @@ -2007,6 +2020,8 @@ static int sd_co_create(BlockdevCreateOptions *options,
> Error **errp)
>
> assert(options->driver == BLOCKDEV_DRIVER_SHEEPDOG);
>
> +deprecation_warning();
> +
> s = g_new0(BDRVSheepdogState, 1);
>
> /* Steal SocketAddress from QAPI, set NULL to prevent double free */
> diff --git a/configure b/configure
> index 7564479008..c6af83f2e6 100755
> --- a/configure
> +++ b/configure
> @@ -533,7 +533,7 @@ vdi="yes"
> vvfat="yes"
> qed="yes"
> parallels="yes"
> -sheepdog="yes"
> +sheepdog="no"
> libxml2=""
> debug_mutex="no"
> libpmem=""
> @@ -1941,7 +1941,7 @@ disabled with --disable-FEATURE, default is enabled if
> available:
>vvfat vvfat image format support
>qed qed image format support
>parallels parallels image format support
> - sheepdogsheepdog block driver support
> + sheepdogsheepdog block driver support (deprecated)
>crypto-afalgLinux AF_ALG crypto backend driver
>capstonecapstone disassembler support
>debug-mutex mutex debugging support
> @@ -7350,6 +7350,7 @@ if test "$parallels" = "yes" ; then
>echo "CONFIG_PARALLELS=y" >> $config_host_mak
> fi
> if test "$sheepdog" = "yes" ; then
> + add_to deprecated_features "sheepdog"
>echo "CONFIG_SHEEPDOG=y" >> $config_host_mak
> fi
> if test "$pty_h" = "yes" ; then
> diff --git a/docs/system/deprecated.rst b/docs/system/deprecated.rst
> index 0cb8b01424..49b9f4b02e 100644
> --- a/docs/system/deprecated.rst
> +++ b/docs/system/deprecated.rst
> @@ -405,6 +405,15 @@ The above, converted to the current supported format::
>
>json:{"file.driver":"rbd", "file.pool":"rbd", "file.image":"name"}
>
> +``sheepdog`` driver (since 5.2.0)
> +^
> +
> +The ``sheepdog`` block device driver is deprecated. The corresponding
> upstream
> +server project is no longer actively maintained. Users are recommended to
> switch
> +to an alternative distributed block device driver such as RBD. The
> +``qemu-img convert`` command can be used to liberate existing data by moving
> +it out of sheepdog volumes into an alternative storage backend.
> +
> linux-user mode CPUs
[libvirt PATCH] util/virgdbus: fix memory leak in virGDBusIsServiceInList
g_variant_iter_loop() handles freeing all arguments unless we break out
of the loop, in that case we have to free them manually.
Reported-by: Peter Krempa
Signed-off-by: Pavel Hrdina
---
src/util/virgdbus.c | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/src/util/virgdbus.c b/src/util/virgdbus.c
index cd9ca8d5d6..4360a6acff 100644
--- a/src/util/virgdbus.c
+++ b/src/util/virgdbus.c
@@ -359,8 +359,10 @@ virGDBusIsServiceInList(const char *listMethod,
g_variant_get(reply, "(as)", &iter);
while (g_variant_iter_loop(iter, "s", &str)) {
-if (STREQ(str, name))
+if (STREQ(str, name)) {
+g_free(str);
return 0;
+}
}
return -2;
--
2.26.2
Re: [libvirt PATCHv2 1/2] tests: build SELinux tests
On Wed, Sep 23, 2020 at 15:39:49 +0200, Ján Tomko wrote: > We set WITH_LIBATTR in meson.build, not WITH_ATTR. > > Signed-off-by: Ján Tomko > Fixes: 3ace72965c3b11fc763f781ae7ce3ca29dd36507 > --- > tests/meson.build | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) I got: [1010/1047] Linking target tests/securityselinuxlabeltest FAILED: tests/securityselinuxlabeltest cc -o tests/securityselinuxlabeltest src/libvirt_probes.o tests/securityselinuxlabeltest.p/securityselinuxlabeltest.c.o -Wl,--as-needed -Wl,--no-undefined -Wl,-export-dynamic -pie -Wl,--whole-archive -Wl,--start-group tests/libtest_utils.a tests/libtest_utils_qemu.a -Wl,--no-whole-archive src/libvirt.so.0.6008.0 -Wl,--no-copy-dt-needed-entries -Wl,-export-dynamic -ldl /usr/lib64/libglib-2.0.so /usr/lib64/libgobject-2.0.so /usr/lib64/libgio-2.0.so /usr/lib64/libgnutls.so /usr/lib64/libnl-3.so /usr/lib64/libnl-route-3.so /usr/lib64/libxml2.so /usr/lib64/libsasl2.so -lselinux /usr/lib64/libtirpc.so /usr/lib64/libyajl.so -Wl,-export-dynamic -lselinux -Wl,-export-dynamic -lselinux -Wl,--end-group '-Wl,-rpath,$ORIGIN/../src' -Wl,-rpath-link,/home/pipo/build/libvirt/gcc/src /bin/ld: tests/securityselinuxlabeltest.p/securityselinuxlabeltest.c.o: in function `mymain': /home/pipo/build/libvirt/gcc/../../../libvirt/tests/securityselinuxlabeltest.c:349: undefined reference to `virQEMUCapsNew' /bin/ld: /home/pipo/build/libvirt/gcc/../../../libvirt/tests/securityselinuxlabeltest.c:352: undefined reference to `virQEMUCapsSet' /bin/ld: /home/pipo/build/libvirt/gcc/../../../libvirt/tests/securityselinuxlabeltest.c:353: undefined reference to `virQEMUCapsSet' /bin/ld: tests/libtest_utils_qemu.a(testutilsqemu.c.o): in function `qemuTestParseCapabilitiesArch': /home/pipo/build/libvirt/gcc/../../../libvirt/tests/testutilsqemu.c:292: undefined reference to `virQEMUCapsNewBinary' /bin/ld: /home/pipo/build/libvirt/gcc/../../../libvirt/tests/testutilsqemu.c:293: undefined reference to `virQEMUCapsLoadCache' /bin/ld: tests/libtest_utils_qemu.a(testutilsqemu.c.o): in function `qemuTestCapsCacheInsert': /home/pipo/build/libvirt/gcc/../../../libvirt/tests/testutilsqemu.c:328: undefined reference to `virQEMUCapsNewCopy' /bin/ld: /home/pipo/build/libvirt/gcc/../../../libvirt/tests/testutilsqemu.c:333: undefined reference to `virQEMUCapsHasMachines' /bin/ld: /home/pipo/build/libvirt/gcc/../../../libvirt/tests/testutilsqemu.c:330: undefined reference to `virQEMUCapsNew' after applying this patch.
Re: [libvirt PATCHv2 2/2] tests: use g_new0 instead of VIR_ALLOC_N
On Wed, Sep 23, 2020 at 15:39:50 +0200, Ján Tomko wrote: > Signed-off-by: Ján Tomko > --- Reviewed-by: Peter Krempa
Re: [libvirt PATCH v2] qemu: substitute missing model name for host-passthrough
On Wed, Sep 23, 2020 at 09:26:58 +0200, Tim Wiederhake wrote:
> From: Collin Walling
>
> Before:
> $ uname -m
> s390x
> $ cat passthrough-cpu.xml
>
> $ virsh hypervisor-cpu-compare passthrough-cpu.xml
> error: Failed to compare hypervisor CPU with passthrough-cpu.xml
> error: internal error: unable to execute QEMU command 'query-cpu-model-comp
> arison': Invalid parameter type for 'modelb.name', expected: string
>
> After:
> $ virsh hypervisor-cpu-compare passthrough-cpu.xml
> CPU described in passthrough-cpu.xml is identical to the CPU provided by hy
> pervisor on the host
>
> Signed-off-by: Tim Wiederhake
> ---
> src/qemu/qemu_driver.c | 9 +
> 1 file changed, 9 insertions(+)
>
> diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
> index ae715c01d7..1cecef01f7 100644
> --- a/src/qemu/qemu_driver.c
> +++ b/src/qemu/qemu_driver.c
> @@ -12336,6 +12336,15 @@ qemuConnectCompareHypervisorCPU(virConnectPtr conn,
> if (virCPUDefParseXMLString(xmlCPU, VIR_CPU_TYPE_AUTO, &cpu) < 0)
> goto cleanup;
>
> +if (!cpu->model) {
> +if (cpu->mode == VIR_CPU_MODE_HOST_PASSTHROUGH) {
> +cpu->model = g_strdup("host");
> +} else {
> +virReportError(VIR_ERR_INVALID_ARG, "%s",
> + _("cpu parameter is missing a model name"));
> +goto cleanup;
> +}
> +}
> ret = qemuConnectCPUModelComparison(qemuCaps, cfg->libDir,
> cfg->user, cfg->group,
> hvCPU, cpu, failIncompatible);
Reviewed-by: Jiri Denemark
I'll wait some time for Collin to add Signed-of-by tag before pushing
this.
Re: [libvirt PATCH] util/virgdbus: fix memory leak in virGDBusIsServiceInList
On Wed, Sep 23, 2020 at 15:57:00 +0200, Pavel Hrdina wrote: > g_variant_iter_loop() handles freeing all arguments unless we break out > of the loop, in that case we have to free them manually. > > Reported-by: Peter Krempa > Signed-off-by: Pavel Hrdina > --- > src/util/virgdbus.c | 4 +++- > 1 file changed, 3 insertions(+), 1 deletion(-) Reviewed-by: Peter Krempa
[libvirt PATCH] virnetdaemon: fix memory leak in virNetDaemonCallInhibit
g_variant_new() returns a weak reference which can be consumed by passing
to other g_variant* functions or to g_dbus_connection_call* functions.
This make it possible to call g_variant_new() directly as argument to
the functions above. Because this might be confusing I explicitly call
g_variant_ref_sink() to make it normal reference in both
virGDBusCallMethod() and virGDBusCallMethodWithFD() so the caller is
always responsible for the data.
Reported-by: Peter Krempa
Signed-off-by: Pavel Hrdina
---
src/rpc/virnetdaemon.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/src/rpc/virnetdaemon.c b/src/rpc/virnetdaemon.c
index f3a5e9f75c..2e01244f74 100644
--- a/src/rpc/virnetdaemon.c
+++ b/src/rpc/virnetdaemon.c
@@ -469,7 +469,7 @@ virNetDaemonCallInhibit(virNetDaemonPtr dmn,
{
g_autoptr(GVariant) reply = NULL;
g_autoptr(GUnixFDList) replyFD = NULL;
-GVariant *message = NULL;
+g_autoptr(GVariant) message = NULL;
GDBusConnection *systemBus;
int fd;
int rc;
--
2.26.2
Re: [libvirt PATCH] virnetdaemon: fix memory leak in virNetDaemonCallInhibit
On a Wednesday in 2020, Pavel Hrdina wrote: g_variant_new() returns a weak reference which can be consumed by passing to other g_variant* functions or to g_dbus_connection_call* functions. This make it possible to call g_variant_new() directly as argument to the functions above. Because this might be confusing I explicitly call g_variant_ref_sink() to make it normal reference in both virGDBusCallMethod() and virGDBusCallMethodWithFD() so the caller is always responsible for the data. Reported-by: Peter Krempa Signed-off-by: Pavel Hrdina --- src/rpc/virnetdaemon.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) Reviewed-by: Ján Tomko Jano signature.asc Description: PGP signature
[libvirt PATCH] util: do not unref event thread after joining it
==295055== Invalid read of size 4
==295055==at 0x4DA4AE4: g_thread_unref (in
/usr/lib64/libglib-2.0.so.0.6400.5)
==295055==by 0x491D5FA: vir_event_thread_finalize (vireventthread.c:47)
==295055==by 0x4E6BCFF: g_object_unref (in
/usr/lib64/libgobject-2.0.so.0.6400.5)
==295055==by 0x22F35CF4: qemuProcessQMPFree (qemu_process.c:8525)
==295055==by 0x22E71B58: glib_autoptr_clear_qemuProcessQMP
(qemu_process.h:237)
...
==295055==by 0x22E98A29: qemuDomainPostParseDataAlloc (qemu_domain.c:5476)
==295055==by 0x49ABF83: virDomainDefPostParse (domain_conf.c:6023)
==295055== Address 0x2acb1c68 is 24 bytes inside a block of size 88 free'd
==295055==at 0x483B9F5: free (vg_replace_malloc.c:538)
==295055==by 0x4D80A4C: g_free (in /usr/lib64/libglib-2.0.so.0.6400.5)
...
==295055==by 0x491D5F1: vir_event_thread_finalize (vireventthread.c:46)
==295055==by 0x4E6BCFF: g_object_unref (in
/usr/lib64/libgobject-2.0.so.0.6400.5)
==295055==by 0x22F35CF4: qemuProcessQMPFree (qemu_process.c:8525)
==295055==by 0x22E71B58: glib_autoptr_clear_qemuProcessQMP
(qemu_process.h:237)
...
==295055== Block was alloc'd at
==295055==at 0x483A809: malloc (vg_replace_malloc.c:307)
==295055==by 0x4D80958: g_malloc (in /usr/lib64/libglib-2.0.so.0.6400.5)
...
==295055==by 0x4DA4C32: g_thread_try_new (in
/usr/lib64/libglib-2.0.so.0.6400.5)
==295055==by 0x491D3BC: virEventThreadStart (vireventthread.c:159)
==295055==by 0x491D3BC: virEventThreadNew (vireventthread.c:185)
...
Signed-off-by: Ján Tomko
Fixes: f4fc3db9204407874181117085756c9ced78adad
---
src/util/vireventthread.c | 1 -
1 file changed, 1 deletion(-)
diff --git a/src/util/vireventthread.c b/src/util/vireventthread.c
index 1bca2aa57a..8342f420f6 100644
--- a/src/util/vireventthread.c
+++ b/src/util/vireventthread.c
@@ -44,7 +44,6 @@ vir_event_thread_finalize(GObject *object)
if (evt->thread) {
g_main_loop_quit(evt->loop);
g_thread_join(evt->thread);
-g_thread_unref(evt->thread);
}
g_main_loop_unref(evt->loop);
--
2.26.2
Re: [libvirt PATCH] virnetdaemon: fix memory leak in virNetDaemonCallInhibit
On Wed, Sep 23, 2020 at 16:37:48 +0200, Pavel Hrdina wrote: > g_variant_new() returns a weak reference which can be consumed by passing > to other g_variant* functions or to g_dbus_connection_call* functions. > > This make it possible to call g_variant_new() directly as argument to > the functions above. Because this might be confusing I explicitly call > g_variant_ref_sink() to make it normal reference in both > virGDBusCallMethod() and virGDBusCallMethodWithFD() so the caller is > always responsible for the data. > > Reported-by: Peter Krempa > Signed-off-by: Pavel Hrdina > --- Reviewed-by: Peter Krempa
Re: [libvirt PATCH] util: do not unref event thread after joining it
On Wed, Sep 23, 2020 at 16:39:13 +0200, Ján Tomko wrote: > ==295055== Invalid read of size 4 ... g_thread_join() eats a reference Reviewed-by: Peter Krempa
[libvirt PATCHv3] tests: build SELinux tests (glib chronicles?)
We set WITH_LIBATTR in meson.build, not WITH_ATTR.
Signed-off-by: Ján Tomko
Fixes: 3ace72965c3b11fc763f781ae7ce3ca29dd36507
---
tests/meson.build | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/tests/meson.build b/tests/meson.build
index 31e8d66c7a..cb720f3afe 100644
--- a/tests/meson.build
+++ b/tests/meson.build
@@ -477,7 +477,7 @@ if conf.has('WITH_REMOTE')
endif
if conf.has('WITH_SECDRIVER_SELINUX')
- if conf.has('WITH_ATTR')
+ if conf.has('WITH_LIBATTR')
tests += [
{ 'name': 'securityselinuxtest' },
{ 'name': 'viridentitytest' },
@@ -485,7 +485,7 @@ if conf.has('WITH_SECDRIVER_SELINUX')
if conf.has('WITH_QEMU')
tests += [
-{ 'name': 'securityselinuxlabeltest', 'link_whole': [
test_utils_qemu_lib ] },
+{ 'name': 'securityselinuxlabeltest', 'link_with': [
test_qemu_driver_lib ], 'link_whole': [ test_utils_qemu_lib ] },
]
endif
endif
--
2.26.2
Re: [libvirt PATCHv3] tests: build SELinux tests (glib chronicles?)
I presume you added the part in the parentheses just for mailing list submission. On Wed, Sep 23, 2020 at 17:05:05 +0200, Ján Tomko wrote: > We set WITH_LIBATTR in meson.build, not WITH_ATTR. Linking fix not mentioned. > Signed-off-by: Ján Tomko > Fixes: 3ace72965c3b11fc763f781ae7ce3ca29dd36507 > --- > tests/meson.build | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) Reviewed-by: Peter Krempa
Re: [PATCH 02/13] virStorageVolDefParseXML: Use g_steal_pointer
On a Wednesday in 2020, Peter Krempa wrote: Signed-off-by: Peter Krempa --- src/conf/storage_conf.c | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) Reviewed-by: Ján Tomko Jano signature.asc Description: PGP signature
Re: [PATCH 01/13] virStorageSourceNew: Abort on failure
On a Wednesday in 2020, Peter Krempa wrote: Add an abort() on the class/object allocation failures so that virStorageSourceNew() always returns a virStorageSource and remove checks from all callers. The allocation APIs in GLib and libvirt already do abort(). All that's left are usage errors. Signed-off-by: Peter Krempa --- src/conf/backup_conf.c| 7 ++ src/conf/domain_conf.c| 21 ++ src/conf/snapshot_conf.c | 7 ++ src/conf/storage_conf.c | 4 +--- src/qemu/qemu_domain.c| 21 ++ src/qemu/qemu_driver.c| 12 +++--- src/qemu/qemu_migration.c | 7 ++ src/qemu/qemu_snapshot.c | 3 +-- src/storage/storage_backend_gluster.c | 5 + src/storage/storage_backend_logical.c | 4 +--- src/storage/storage_util.c| 10 +++-- src/util/virstoragefile.c | 32 ++- tests/qemublocktest.c | 16 +++--- tests/virstoragetest.c| 5 + 14 files changed, 46 insertions(+), 108 deletions(-) Reviewed-by: Ján Tomko Jano signature.asc Description: PGP signature
Re: [PATCH 03/13] qemuDomainBlockRebase: Replace ternary operator with if/else
On a Wednesday in 2020, Peter Krempa wrote: Signed-off-by: Peter Krempa --- src/qemu/qemu_driver.c | 6 -- 1 file changed, 4 insertions(+), 2 deletions(-) Reviewed-by: Ján Tomko Jano signature.asc Description: PGP signature
Re: [PATCH 04/13] qemuSnapshotCreateInactiveExternal: Don't access 'idx' of snapshot
On a Wednesday in 2020, Peter Krempa wrote: After virDomainSnapshotAlignDisks is called the definitions of disks in the snapshot definition and in the domain definition are in the same order so they can be addressed using the same index. This frees up 'idx' to be removed later. Signed-off-by: Peter Krempa --- src/qemu/qemu_snapshot.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) Reviewed-by: Ján Tomko Jano signature.asc Description: PGP signature
Re: [PATCH 05/13] virDomainSnapshotAlignDisks: Refactor cleanup
On a Wednesday in 2020, Peter Krempa wrote: Use automatic pointer for the bitmap and get rid of the 'cleanup' label and 'ret' variable. Signed-off-by: Peter Krempa --- src/conf/snapshot_conf.c | 33 + 1 file changed, 13 insertions(+), 20 deletions(-) Reviewed-by: Ján Tomko Jano signature.asc Description: PGP signature
Re: [PATCH 06/13] virDomainSnapshotAlignDisks: Rename 'def' -> 'snapdef'
On a Wednesday in 2020, Peter Krempa wrote: While this function resides in the snapshot config module the 'def' , the variable is referencing the VM definition in most places. Change the name to 'snapdef' to avoid ambiguity especially since we are also dealing with the domain definition in this function. Signed-off-by: Peter Krempa --- src/conf/snapshot_conf.c | 42 1 file changed, 21 insertions(+), 21 deletions(-) Reviewed-by: Ján Tomko Jano signature.asc Description: PGP signature
Re: [PATCH 11/13] virDomainSnapshotAlignDisks: clarify handing of snapshot location
On 9/23/20 8:33 AM, Peter Krempa wrote: Remove the use of the 'disk_snapshot' temporary variable since accessing the disk definition now isn't that much longer to write and use expicit explicit value checks instead of the (non-)zero check to make it more obvious what the code is doing. Signed-off-by: Peter Krempa --- src/conf/snapshot_conf.c | 16 1 file changed, 8 insertions(+), 8 deletions(-) Reviewed-by: Eric Blake -- Eric Blake, Principal Software Engineer Red Hat, Inc. +1-919-301-3226 Virtualization: qemu.org | libvirt.org
Re: [PATCH 07/13] virDomainSnapshotAlignDisks: Rename 'disk' -> 'snapdisk'
On a Wednesday in 2020, Peter Krempa wrote: The 'disk' variable usually refers to a definition of a disk from the domain definition. Rename it to 'snapdisk' to be clear that we are Sounds catchy. Have you bought the domain already? talking about the snapshot disk definition especially since this function also accesses the domain disk definition. Signed-off-by: Peter Krempa --- src/conf/snapshot_conf.c | 54 1 file changed, 27 insertions(+), 27 deletions(-) Reviewed-by: Ján Tomko Jano signature.asc Description: PGP signature
Re: [PATCH 08/13] virDomainSnapshotAlignDisks: Add 'domdef' local variable
On a Wednesday in 2020, Peter Krempa wrote: There are multiple places accessing the domain definition. Extract it to a local variable so that it's more clear what's happening. Signed-off-by: Peter Krempa --- src/conf/snapshot_conf.c | 27 ++- 1 file changed, 14 insertions(+), 13 deletions(-) Reviewed-by: Ján Tomko Jano signature.asc Description: PGP signature
Re: [PATCH 12/13] virDomainSnapshotAlignDisks: refactor extension to all disks
On 9/23/20 8:33 AM, Peter Krempa wrote: Last step of the algorithm in virDomainSnapshotAlignDisks is to extend the array of disks to all VM's disk and provide defaults. This was done by extending the array, adding defaults at the end and then sorting it. This requires the 'idx' variable and also a separate sorting function. If we store the pointer to existing snapshot disk definitions in a hash table and create a new array of snapshot disk definitions, we can fill the new array directly by either copying the definition from the old array or adding the default. This avoids the sorting step and thus even the need to store the index of the domain disk altogether. Signed-off-by: Peter Krempa --- src/conf/snapshot_conf.c | 48 +++- 1 file changed, 18 insertions(+), 30 deletions(-) Nice cleanup. Reviewed-by: Eric Blake -- Eric Blake, Principal Software Engineer Red Hat, Inc. +1-919-301-3226 Virtualization: qemu.org | libvirt.org
Re: [PATCH 09/13] virDomainSnapshotAlignDisks: Extract domain disk definition to a local variable
On a Wednesday in 2020, Peter Krempa wrote: Extract the disk def to a local variable so that it's more obvious what's happening and it will also allow further simplification. Signed-off-by: Peter Krempa --- src/conf/snapshot_conf.c | 9 ++--- 1 file changed, 6 insertions(+), 3 deletions(-) Reviewed-by: Ján Tomko Jano signature.asc Description: PGP signature
Re: [PATCH 10/13] virDomainSnapshotAlignDisks: remove unnecessary 'tmp' variable
On a Wednesday in 2020, Peter Krempa wrote: The converted string is used exactly once so we can call the conversion without storing the result in a variable. I presume this was done to reduce line length, not to reuse it. Signed-off-by: Peter Krempa --- src/conf/snapshot_conf.c | 6 ++ 1 file changed, 2 insertions(+), 4 deletions(-) Reviewed-by: Ján Tomko Jano signature.asc Description: PGP signature
Re: [PATCH 11/13] virDomainSnapshotAlignDisks: clarify handing of snapshot location
On a Wednesday in 2020, Peter Krempa wrote: Remove the use of the 'disk_snapshot' temporary variable since accessing the disk definition now isn't that much longer to write and use expicit explicit value checks instead of the (non-)zero check to make it more obvious what the code is doing. Signed-off-by: Peter Krempa --- src/conf/snapshot_conf.c | 16 1 file changed, 8 insertions(+), 8 deletions(-) Reviewed-by: Ján Tomko Jano signature.asc Description: PGP signature
Re: [PATCH 12/13] virDomainSnapshotAlignDisks: refactor extension to all disks
On a Wednesday in 2020, Peter Krempa wrote: Last step of the algorithm in virDomainSnapshotAlignDisks is to extend the array of disks to all VM's disk and provide defaults. This was done by extending the array, adding defaults at the end and then sorting it. This requires the 'idx' variable and also a separate sorting function. If we store the pointer to existing snapshot disk definitions in a hash table and create a new array of snapshot disk definitions, we can fill the new array directly by either copying the definition from the old array or adding the default. This avoids the sorting step and thus even the need to store the index of the domain disk altogether. Signed-off-by: Peter Krempa --- src/conf/snapshot_conf.c | 48 +++- 1 file changed, 18 insertions(+), 30 deletions(-) Reviewed-by: Ján Tomko Jano signature.asc Description: PGP signature
Re: [PATCH 13/13] virDomainSnapshotDiskDef: Remove 'idx' field
On a Wednesday in 2020, Peter Krempa wrote: It's no longer needed and is valid only after virDomainSnapshotAlignDisks is called while holding the lock. Signed-off-by: Peter Krempa --- src/conf/snapshot_conf.h | 1 - 1 file changed, 1 deletion(-) Reviewed-by: Ján Tomko Jano signature.asc Description: PGP signature
Re: [PATCH] apparmor: Allow /usr/libexec for libxl-save-helper and pygrub
On 9/23/20 7:51 AM, Jim Fehlig wrote: On 9/23/20 7:26 AM, Christian Ehrhardt wrote: On Wed, Sep 23, 2020 at 12:35 AM Jim Fehlig wrote: Like other distros, openSUSE Tumbleweed recently changed libexecdir from /usr/lib to /usr/libexec. Add it as an allowed path for libxl-save-helper and pygrub. Hi Jim, ack to the intention, but I think since this should use @libexecdir@ I think. Or did anything change that this doesn't apply anymore ... in that case I beg your pardon. [1]: https://libvirt.org/git/?p=libvirt.git;a=commit;h=5c8bd31c881e99261ac098e867a79b300440731a Heh, I see that skipped over the xen stuff :-). I'll send a V2 later. Thinking about it more, perhaps it is best to go with this V1 patch since these are not files provided by libvirt but xen, where conceivably libvirt and xen could be built with different libexecdir? IMO it would be best to explicitly list the known paths distros have used for libxl-save-helper and pygrub. Regards, Jim
Re: [PATCH] xen: Don't add dom0 twice on driver reload
On a Friday in 2020, Jim Fehlig wrote: When the xen driver loads, it probes libxl for some info about dom0 and adds it to the virDomainObjList. The driver then looks for any domains in stateDir and if they are still alive adds them to the list as well. This logic is a bit flawed wrt handling driver reload and causes the following error internal error: unexpected domain Domain-0 already exists A simple fix is to load all domains from stateDir first and then only add dom0 if needed. Signed-off-by: Jim Fehlig --- src/libxl/libxl_driver.c | 46 +++- 1 file changed, 26 insertions(+), 20 deletions(-) Reviewed-by: Ján Tomko Jano signature.asc Description: PGP signature
Re: [PATCH V3] Modify virCPUarmCompare to perform compare actions
On Wed, Sep 16, 2020 at 16:58:03 +0800, 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;
This looks a bit too complicated as there's no common code to be
executed for several ret values. Just drop this variable and return
directly wherever you set it.
> +
> +/* 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)) {
Use STRNEQ_NULLABLE instead.
> +VIR_DEBUG("Host CPU model does not match required CPU model %s",
> + cpu->model);
NULLSTR(cpu->model)
and I would also add NULLSTR(cpu->vendor) in the message just in case
the CPUs differ only in vendor.
> +
> +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
Jirka
Re: [PATCH] apparmor: Allow /usr/libexec for libxl-save-helper and pygrub
On Wed, Sep 23, 2020 at 12:46 PM Jim Fehlig wrote: > > On 9/23/20 7:51 AM, Jim Fehlig wrote: > > On 9/23/20 7:26 AM, Christian Ehrhardt wrote: > >> On Wed, Sep 23, 2020 at 12:35 AM Jim Fehlig wrote: > >>> > >>> Like other distros, openSUSE Tumbleweed recently changed libexecdir from > >>> /usr/lib to /usr/libexec. Add it as an allowed path for libxl-save-helper > >>> and pygrub. > >> > >> Hi Jim, > >> ack to the intention, but I think since this should use @libexecdir@ I > >> think. > >> Or did anything change that this doesn't apply anymore ... in that > >> case I beg your pardon. > >> > >> [1]: > >> https://libvirt.org/git/?p=libvirt.git;a=commit;h=5c8bd31c881e99261ac098e867a79b300440731a > >> > > > > Heh, I see that skipped over the xen stuff :-). I'll send a V2 later. > > Thinking about it more, perhaps it is best to go with this V1 patch since > these > are not files provided by libvirt but xen, where conceivably libvirt and xen > could be built with different libexecdir? IMO it would be best to explicitly > list the known paths distros have used for libxl-save-helper and pygrub. > It is entirely possible that one has not been updated yet, or someone is mixing packages, so this patch makes sense over having it assume a specific path. -- 真実はいつも一つ！/ Always, there's only one truth!
Re: [PATCH] apparmor: Allow /usr/libexec for libxl-save-helper and pygrub
On Tue, Sep 22, 2020 at 6:35 PM Jim Fehlig wrote:
>
> Like other distros, openSUSE Tumbleweed recently changed libexecdir from
> /usr/lib to /usr/libexec. Add it as an allowed path for libxl-save-helper
> and pygrub.
>
> Signed-off-by: Jim Fehlig
> ---
>
> I considered including /usr/lib64, but I don't think any distros are
> installing xen libexecdir targets to /usr/lib64. Happy to include it
> if I'm wrong :-).
>
> src/security/apparmor/usr.sbin.libvirtd.in | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/src/security/apparmor/usr.sbin.libvirtd.in
> b/src/security/apparmor/usr.sbin.libvirtd.in
> index f2030764cd..bf4563e1e8 100644
> --- a/src/security/apparmor/usr.sbin.libvirtd.in
> +++ b/src/security/apparmor/usr.sbin.libvirtd.in
> @@ -86,8 +86,8 @@ profile libvirtd @sbindir@/libvirtd
> flags=(attach_disconnected) {
>/{usr/,}lib/udev/scsi_id PUx,
>/usr/{lib,lib64}/xen-common/bin/xen-toolstack PUx,
>/usr/{lib,lib64}/xen/bin/* Ux,
> - /usr/lib/xen-*/bin/libxl-save-helper PUx,
> - /usr/lib/xen-*/bin/pygrub PUx,
> + /usr/{lib,libexec}/xen-*/bin/libxl-save-helper PUx,
> + /usr/{lib,libexec}/xen-*/bin/pygrub PUx,
>/usr/{lib,lib64,lib/qemu,libexec}/vhost-user-gpu PUx,
>/usr/{lib,lib64,lib/qemu,libexec}/virtiofsd PUx,
>
> --
> 2.28.0
>
Yay! Looks great to me!
Reviewed-by: Neal Gompa
--
真実はいつも一つ！/ Always, there's only one truth!
[libvirt PATCH 02/14] vbox: remove VBoxCGlueTerm
cppcheck reports:
src/vbox/vbox_XPCOMCGlue.c:226:21: style:
The statement 'if (hVBoxXPCOMC!=NULL) hVBoxXPCOMC=NULL' is
logically equivalent to 'hVBoxXPCOMC=NULL'.
[duplicateConditionalAssign]
It does not matter anyway because this function
is never called.
Fixes: e1506cb4eb7eab96e7ded27a23f0d8ac9697ac2a
Signed-off-by: Ján Tomko
---
src/vbox/vbox_XPCOMCGlue.c | 19 ---
src/vbox/vbox_XPCOMCGlue.h | 1 -
2 files changed, 20 deletions(-)
diff --git a/src/vbox/vbox_XPCOMCGlue.c b/src/vbox/vbox_XPCOMCGlue.c
index 3cbb679110..2936ff0edb 100644
--- a/src/vbox/vbox_XPCOMCGlue.c
+++ b/src/vbox/vbox_XPCOMCGlue.c
@@ -217,25 +217,6 @@ VBoxCGlueInit(unsigned int *version)
}
-/**
- * Terminate the C glue library.
- */
-void
-VBoxCGlueTerm(void)
-{
-if (hVBoxXPCOMC != NULL) {
-#if 0 /* VBoxRT.so doesn't like being reloaded. See @bugref{3725}. */
-dlclose(g_hVBoxXPCOMC);
-#endif
-hVBoxXPCOMC = NULL;
-}
-
-pVBoxFuncs_v2_2 = NULL;
-g_pfnGetFunctions = NULL;
-}
-
-
-
/*
* In XPCOM an array is represented by 1) a pointer to an array of pointers
* that point to the items and 2) an unsigned int representing the number of
diff --git a/src/vbox/vbox_XPCOMCGlue.h b/src/vbox/vbox_XPCOMCGlue.h
index 517b02393f..d0e579482e 100644
--- a/src/vbox/vbox_XPCOMCGlue.h
+++ b/src/vbox/vbox_XPCOMCGlue.h
@@ -36,7 +36,6 @@
extern PFNVBOXGETXPCOMCFUNCTIONS g_pfnGetFunctions;
int VBoxCGlueInit(unsigned int *version);
-void VBoxCGlueTerm(void);
typedef struct _vboxArray vboxArray;
--
2.26.2
[libvirt PATCH 05/14] Remove pointless initializations
Signed-off-by: Ján Tomko
---
src/test/test_driver.c | 2 +-
tests/virnumamock.c| 2 +-
tests/virrandommock.c | 2 +-
3 files changed, 3 insertions(+), 3 deletions(-)
diff --git a/src/test/test_driver.c b/src/test/test_driver.c
index d582c207f4..cbbfea6665 100644
--- a/src/test/test_driver.c
+++ b/src/test/test_driver.c
@@ -4354,7 +4354,7 @@ testNodeGetFreePages(virConnectPtr conn G_GNUC_UNUSED,
unsigned long long *counts,
unsigned int flags)
{
-size_t i = 0, j = 0;
+size_t i, j;
int x = 6;
virCheckFlags(0, -1);
diff --git a/tests/virnumamock.c b/tests/virnumamock.c
index 40e18e646e..d39c264a3f 100644
--- a/tests/virnumamock.c
+++ b/tests/virnumamock.c
@@ -130,7 +130,7 @@ virNumaGetPages(int node,
{
const int pages_def[] = { 4, 2 * 1024, 1 * 1024 * 1024};
const int npages_def = G_N_ELEMENTS(pages_def);
-size_t i = 0;
+size_t i;
if (pages_size)
*pages_size = g_new0(unsigned int, npages_def);
diff --git a/tests/virrandommock.c b/tests/virrandommock.c
index 6dd15213e3..ca0520a5a3 100644
--- a/tests/virrandommock.c
+++ b/tests/virrandommock.c
@@ -69,7 +69,7 @@ int
gnutls_dh_params_generate2(gnutls_dh_params_t dparams,
unsigned int bits)
{
-int rc = 0;
+int rc;
VIR_MOCK_REAL_INIT(gnutls_dh_params_generate2);
--
2.26.2
[libvirt PATCH 13/14] storage: storageBackendWipeLocal: reduce variable scope
Also use MIN instead of open-coding it.
Signed-off-by: Ján Tomko
---
src/storage/storage_util.c | 6 ++
1 file changed, 2 insertions(+), 4 deletions(-)
diff --git a/src/storage/storage_util.c b/src/storage/storage_util.c
index 49ecbc5344..23632fc884 100644
--- a/src/storage/storage_util.c
+++ b/src/storage/storage_util.c
@@ -2526,10 +2526,8 @@ storageBackendWipeLocal(const char *path,
size_t writebuf_length,
bool zero_end)
{
-int written = 0;
unsigned long long remaining = 0;
off_t size;
-size_t write_size = 0;
g_autofree char *writebuf = NULL;
if (VIR_ALLOC_N(writebuf, writebuf_length) < 0)
@@ -2557,9 +2555,9 @@ storageBackendWipeLocal(const char *path,
remaining = wipe_len;
while (remaining > 0) {
+size_t write_size = MIN(writebuf_length, remaining);
+int written = safewrite(fd, writebuf, write_size);
-write_size = (writebuf_length < remaining) ? writebuf_length :
remaining;
-written = safewrite(fd, writebuf, write_size);
if (written < 0) {
virReportSystemError(errno,
_("Failed to write %zu bytes to "
--
2.26.2
