Re: [PATCH RESEND 00/20] handle missing SR-IOV VF hostdevs during running domains
On 2/16/21 8:16 AM, Daniel Henrique Barboza wrote: Ping for reviews (patches 1-4 and 7-8 already pushed). Yeah, sorry. It's been on my list ever since I left it half-finished, but I keep getting distra... OOHH LOOK!!! SOMETHING SHINY!! Okay, back to the subject - I promise I will go back to these before today is finished!!
Re: [libvirt PATCH 4/4] ci: Introduce a new checker script 'check-registry.py'
On Wed, 2021-02-10 at 18:24 +0100, Erik Skultety wrote:
> +def get_undesirables(registry_distros_d, hosts_l):
> +""" Returns a dictionary of 'id':'name' pairs of images that can be
> purged"""
Is the var_d and var_l a Python convention that I'm not aware of? I
don't think we're using it anywhere in either libvirt or libvirt-ci,
so I'd rather avoid it here as well.
I see you're using some type hints for the get_image_distro()
function, though. Can we perhaps have more of that?
Suggestion: "stale" is both shorter and harder to misspell than
"undesirable" :)
> +def main():
> +parser = argparse.ArgumentParser()
> +parser.add_argument("project_id",
> +help="GitLab project ID")
Is passing the project ID necessary? We're hardcoding the one for
libvirt/libvirt in util.py and we don't provide a way to override it
for ci-list-images, so I think this can be skipped.
Having a way for the developer to point to their own fork rather than
libvirt/libvirt makes sense, but in that case we probably also want
to make it so the user passes "myusername/libvirt" and we convert
that to a project ID using the GitLab API under the hood.
> +parser.add_argument("--lcitool_path",
> +dest="lcitool",
> +metavar="PATH",
> +help="absolute path to lcitool, CWD is used if
> omitted")
Underscores in command line arguments are inconvenient and ugly, so
please don't use them. You can just call the option "--lcitool".
Regarding the documentation for the option, I'm not convinced it's
accurate: AFAICT shutil.which() will use $PATH, not $CWD, to look for
the named binary if an absolute path to it has not been provided.
> +uri = util.get_registry_uri(util.PROJECT_ID)
> +images_json = util.list_images(uri + "?per_page=100")
We have "?per_page=100" hardcoded in two places now, so it looks like
we can probably just fold its usage into list_images()?
> +sys.exit(f"""
> +The following images can be purged from the registry:
> +
> +{undesirable_image_names}
I would like to see the image ID printed along with the image name,
so that I can tweak the shell snippet below to selectively remove
only *some* of the images.
> +You can remove the above images over the API with the following code snippet:
> +
> +\t$ for image_id in {undesirable_image_ids} \\
> +\t;do \\
Having the "do" on a separate line is weird.
Also please indent using either two or four spaces rather than tabs.
Aside from the (mostly minor) issues that I've pointed out, I think
these patches go in the right direction. However, reviewing them made
me wonder what the long-term status of these scripts should be.
Right now we have the following entry points:
* a Makefile, which can be used to kick off local builds;
* a couple of 'refresh' shell script that are used to regenerate
contents derived from the libvirt-ci source of truth;
* some Python bits that are used by both.
We also know that, at some point in the hopefully not too distant
future, we'll have the results of
https://gitlab.com/libvirt/libvirt-ci/-/merge_requests/59
available, and that will allow us to turn the refresh scripts into
trivial wrappers.
I think it would make sense to converge towards having a single
Python based entry point, which serves as the counterpart of lcitool,
and which can be used to do all of the above.
We can even get there gradually: the first version of the Python
script could just call out to the Makefile to start builds, and then
we could reimplement the logic in Python at a later point in time.
What do you think?
--
Andrea Bolognani / Red Hat / Virtualization
Re: [libvirt PATCH v4 11/25] nodedev: handle mdevs that disappear from mdevctl
On Wed, Feb 03, 2021 at 11:38:55AM -0600, Jonathon Jongsma wrote:
> mdevctl does not currently provide any events when the list of defined
> devices changes, so we will need to poll mdevctl for the list of defined
> devices periodically. When a mediated device no longer exists from one
> iteration to the next, we need to treat it as an "undefine" event.
>
> When we get such an event, we remove the device from the list if it's
> not active. Otherwise, we simply mark it as non-persistent.
>
> Signed-off-by: Jonathon Jongsma
> ---
> src/node_device/node_device_driver.c | 55 +---
> 1 file changed, 50 insertions(+), 5 deletions(-)
>
> diff --git a/src/node_device/node_device_driver.c
> b/src/node_device/node_device_driver.c
> index fd57dcacc1..598cd865c5 100644
> --- a/src/node_device/node_device_driver.c
> +++ b/src/node_device/node_device_driver.c
> @@ -1182,24 +1182,63 @@ struct _virMdevctlForEachData {
> };
>
>
> +/* This function keeps the list of persistent mediated devices consistent
> + * between the nodedev driver and mdevctl.
> + * @obj is a device that is currently known by the nodedev driver, and
> @opaque
> + * contains the most recent list of devices defined by mdevctl. If @obj is no
> + * longer defined in mdevctl, remove it from the driver as well. */
> +static void
> +removeMissingPersistentMdevs(virNodeDeviceObjPtr obj,
> + const void *opaque)
> +{
> +const virMdevctlForEachData *data = opaque;
> +size_t i;
> +virNodeDeviceDefPtr def = virNodeDeviceObjGetDef(obj);
> +virObjectEventPtr event;
> +
> +/* transient mdevs are populated via udev, so don't remove them from the
> + * nodedev driver just because they are not reported by by mdevctl */
> +if (!virNodeDeviceObjIsPersistent(obj))
> +return;
> +
> +for (i = 0; i < data->ndefs; i++) {
> +/* OK, this mdev is still defined by mdevctl */
> +if (STREQ(data->defs[i]->name, def->name))
> +return;
> +}
> +
> +event = virNodeDeviceEventLifecycleNew(def->name,
> + VIR_NODE_DEVICE_EVENT_UNDEFINED,
> + 0);
> +
> +/* The device is active, but no longer defined by mdevctl. Keep the
> device
> + * in the list, but mark it as non-persistent */
> +if (virNodeDeviceObjIsActive(obj))
> +virNodeDeviceObjSetPersistent(obj, false);
> +else
> +virNodeDeviceObjListRemoveLocked(driver->devs, obj);
> +
> +virObjectEventStateQueue(driver->nodeDeviceEventState, event);
> +}
> +
> +
> int
> nodeDeviceUpdateMediatedDevices(void)
> {
> -g_autofree virNodeDeviceDefPtr *defs = NULL;
> -int ndefs;
> +virMdevctlForEachData data = { 0, };
> GList * tofree = NULL;
> size_t i;
>
> -if ((ndefs = virMdevctlListDefined(&defs)) < 0) {
> +if ((data.ndefs = virMdevctlListDefined(&data.defs)) < 0) {
> virReportSystemError(errno, "%s",
> _("failed to query mdevs from mdevctl"));
> return -1;
> }
>
> -for (i = 0; i < ndefs; i++) {
> +for (i = 0; i < data.ndefs; i++) {
> virNodeDeviceObjPtr obj;
> virObjectEventPtr event;
> -virNodeDeviceDefPtr def = defs[i];
> +virNodeDeviceDefPtr def = data.defs[i];
> bool was_defined = false;
>
> def->driver = g_strdup("vfio_mdev");
> @@ -1245,8 +1284,14 @@ nodeDeviceUpdateMediatedDevices(void)
> virObjectEventStateQueue(driver->nodeDeviceEventState, event);
> }
>
> +/* Any mdevs that were previously defined but were not returned the
> latest
> + * mdevctl query should be removed from the device list */
> +virNodeDeviceObjListForEachSafe(driver->devs,
> removeMissingPersistentMdevs,
> +&data);
So, apparently it is now suggested to limit the usage of virHashX to only
virHashNew and resort to the GLib primitives otherwise, which means the code
will have to be adjusted for that. Apart from that I don't see an issue with
this patch.
Reviewed-by: Erik Skultety
(I will still give it a test for aspects that are not immediately clear from
the code)
Re: [libvirt PATCH v4 10/25] nodedev: add helper functions to remove node devices
...
> +void
> +virNodeDeviceObjListForEachSafe(virNodeDeviceObjListPtr devs,
> +virNodeDeviceObjListIterator iter,
> +const void *opaque)
> +{
> +struct _virNodeDeviceObjListForEachData data = {
> +.iter = iter,
> +.opaque = opaque
> +};
> +
> +virObjectRWLockWrite(devs);
> +virHashForEachSafe(devs->objs, virNodeDeviceObjListForEachCb, &data);
I just checked out virHashForEachSafe and virHashForEach says the latter is
deprecated while the use of the former in new code should be rewritten with
GLib, which translates to the use of g_hash_table_foreach_remove().
Erik
Re: [libvirt PATCH 2/4] ci: Rewrite list-images from Bash to Python
On Tue, Feb 16, 2021 at 02:40:32PM +0100, Andrea Bolognani wrote:
> On Wed, 2021-02-10 at 18:24 +0100, Erik Skultety wrote:
> > +# skip the "ci-" prefix each of our container images' name has
> > +names = [i["name"][3:] for i in data]
>
> I would prefer something like
>
> PREFIX = "ci-"
> names = [i["name"][len(PREFIX):] for i in data]
>
> here, rather than hardcoding the length of the string.
>
> > +names_native = [name for name in names if "cross" not in name]
> > +names_cross = [name for name in names if "cross" in name]
> > +
> > +print("Available x86 container images:\n")
> > +print("\t" + "\n\t".join(names_native))
>
> Please use two or four spaces instead of tabs.
Sure, will do.
>
> More high-level feedback about the patch:
>
> * you should drop the sh implementation at the same time as you're
> adding the python one;
>
> * adding this standalone script in one patch only to refactor most
> of its code away to a separate module in the next one leads to
> unnecessary churn and more work for the reviewer: please just add
> the script in its intended form in the first place.
As you wish, I approached the way I'd appreciate it - introduce a drop 1:1
replacement and then change individual bits later...oh well, all of us have
different taste.
Thanks,
Erik
Re: [libvirt PATCH 1/3] conf: add support for VNC power control setting
On a Tuesday in 2021, Daniel P. Berrangé wrote:
The option instructs the
VNC server to enable an extension that lets the client perform a
graceful shutdown, reboot and hard reset.
This is enabled by default since it cannot be assumed that the VNC
client user has administrator rights over the guest OS. In the case
where the VNC user is a guest administrator though, it is reasonable
to allow direct power control host side too.
Signed-off-by: Daniel P. Berrangé
---
docs/formatdomain.rst | 5 +
docs/schemas/domaincommon.rng | 5 +
src/conf/domain_conf.c| 12
src/conf/domain_conf.h| 1 +
4 files changed, 23 insertions(+)
diff --git a/docs/formatdomain.rst b/docs/formatdomain.rst
index eafd6b3396..580319365c 100644
--- a/docs/formatdomain.rst
+++ b/docs/formatdomain.rst
@@ -5791,6 +5791,11 @@ interaction with the admin.
``autoport`` having no effect due to security reasons) :since:`Since
1.0.6` .
+ For VNC, the ``powerControl`` attribute can be used to enable VM
shutdown,
+ reboot and reset power control features for the VNC client. This is
+ appropriate if the authenticated VNC client user already has
administrator
+ privileges in the guest :since:`Since 7.1.0`.
+
Although VNC doesn't support OpenGL natively, it can be paired with
graphics type ``egl-headless`` (see below) which will instruct QEMU to
open and use drm nodes for OpenGL rendering.
diff --git a/docs/schemas/domaincommon.rng b/docs/schemas/domaincommon.rng
index e6de934456..49dc4b5130 100644
--- a/docs/schemas/domaincommon.rng
+++ b/docs/schemas/domaincommon.rng
@@ -3663,6 +3663,11 @@
+
+
+
+
+
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index b731744f04..91933bf292 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -13150,6 +13150,7 @@ virDomainGraphicsDefParseXMLVNC(virDomainGraphicsDefPtr
def,
g_autofree char *websocketGenerated = virXMLPropString(node,
"websocketGenerated");
g_autofree char *sharePolicy = virXMLPropString(node, "sharePolicy");
g_autofree char *autoport = virXMLPropString(node, "autoport");
+g_autofree char *powerControl = virXMLPropString(node, "powerControl");
if (virDomainGraphicsListensParseXML(def, node, ctxt, flags) < 0)
return -1;
@@ -13206,6 +13207,13 @@
virDomainGraphicsDefParseXMLVNC(virDomainGraphicsDefPtr def,
}
}
+if (powerControl &&
+virStringParseYesNo(powerControl, &def->data.vnc.powerControl) < 0) {
+virReportError(VIR_ERR_INTERNAL_ERROR,
+ _("cannot parse vnc power control '%s'"), powerControl);
+return -1;
+}
+
def->data.vnc.keymap = virXMLPropString(node, "keymap");
if (virDomainGraphicsAuthDefParseXML(node, &def->data.vnc.auth,
@@ -27148,6 +27156,10 @@ virDomainGraphicsDefFormat(virBufferPtr buf,
virDomainGraphicsVNCSharePolicyTypeToString(
def->data.vnc.sharePolicy));
+if (def->data.vnc.powerControl)
+virBufferAsprintf(buf, " powerControl='%s'",
+ def->data.vnc.powerControl ? "yes" : "no");
The "no" branch is dead code. If you don't want to format "no" unless it
was explicitly requested, use virTristateBool.
Jano
+
virDomainGraphicsAuthDefFormatAttr(buf, &def->data.vnc.auth, flags);
break;
diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h
index 930eed60de..544ec1b2fa 100644
--- a/src/conf/domain_conf.h
+++ b/src/conf/domain_conf.h
@@ -1712,6 +1712,7 @@ struct _virDomainGraphicsDef {
char *keymap;
virDomainGraphicsAuthDef auth;
int sharePolicy;
+bool powerControl;
} vnc;
struct {
char *display;
--
2.29.2
signature.asc
Description: PGP signature
Re: [libvirt PATCH 3/3] qemu: wire up support for VNC power control options
On Tue, Feb 16, 2021 at 14:08:52 +, Daniel Berrange wrote:
> This allows the VNC client user to perform a shutdown, reboot and reset
> of the VM from the host side.
>
> Signed-off-by: Daniel P. Berrangé
> ---
> src/qemu/qemu_command.c | 9 +
> tests/qemuxml2argvdata/graphics-vnc-policy.args | 2 +-
> tests/qemuxml2argvdata/graphics-vnc-policy.xml | 2 +-
> tests/qemuxml2argvtest.c| 2 +-
> 4 files changed, 12 insertions(+), 3 deletions(-)
>
> diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
> index d801018aa2..266cf7332e 100644
> --- a/src/qemu/qemu_command.c
> +++ b/src/qemu/qemu_command.c
> @@ -7700,6 +7700,15 @@ qemuBuildGraphicsVNCCommandLine(virQEMUDriverConfigPtr
> cfg,
> /* TODO: Support ACLs later */
> }
>
> +if (graphics->data.vnc.powerControl) {
> +if (!virQEMUCapsGet(qemuCaps, QEMU_CAPS_VNC_POWER_CONTROL)) {
> +virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
> + _("VNC power control is not available"));
> +return -1;
> +}
Invoking this check from qemuValidateDomainDeviceDefGraphics will give
you define-time check whether the VM supports this rather than
startup-time.
> +virBufferAddLit(&opt, ",power-control=on");
> +}
So we don't want to be able to explicitly turn this off?
> +
> virCommandAddArg(cmd, "-vnc");
> virCommandAddArgBuffer(cmd, &opt);
> if (graphics->data.vnc.keymap)
[...]
> diff --git a/tests/qemuxml2argvtest.c b/tests/qemuxml2argvtest.c
> index db438c5466..b4df042fea 100644
> --- a/tests/qemuxml2argvtest.c
> +++ b/tests/qemuxml2argvtest.c
> @@ -1364,7 +1364,7 @@ mymain(void)
> QEMU_CAPS_VNC,
> QEMU_CAPS_DEVICE_CIRRUS_VGA);
> DO_TEST("graphics-vnc-policy", QEMU_CAPS_VNC,
> -QEMU_CAPS_DEVICE_CIRRUS_VGA);
> +QEMU_CAPS_DEVICE_CIRRUS_VGA, QEMU_CAPS_VNC_POWER_CONTROL);
Rather than adding the capability, either convert the test case to
DO_TEST_CAPS_LATEST or add a separate _LATEST() case for it.
> DO_TEST("graphics-vnc-no-listen-attr", QEMU_CAPS_VNC,
> QEMU_CAPS_DEVICE_CIRRUS_VGA);
> DO_TEST("graphics-vnc-remove-generated-socket", QEMU_CAPS_VNC,
> --
> 2.29.2
>
Re: [libvirt PATCH 2/3] qemu: probe for -vnc power-control option support
On Tue, Feb 16, 2021 at 14:08:51 +, Daniel Berrange wrote: > Signed-off-by: Daniel P. Berrangé > --- > src/qemu/qemu_capabilities.c | 2 ++ > src/qemu/qemu_capabilities.h | 1 + > tests/qemucapabilitiesdata/caps_6.0.0.x86_64.xml | 1 + > 3 files changed, 4 insertions(+) Reviewed-by: Peter Krempa
Re: [libvirt PATCH 1/3] conf: add support for VNC power control setting
On Tue, Feb 16, 2021 at 14:08:50 +, Daniel Berrange wrote:
> The option instructs the
> VNC server to enable an extension that lets the client perform a
> graceful shutdown, reboot and hard reset.
>
> This is enabled by default since it cannot be assumed that the VNC
> client user has administrator rights over the guest OS. In the case
> where the VNC user is a guest administrator though, it is reasonable
> to allow direct power control host side too.
>
> Signed-off-by: Daniel P. Berrangé
> ---
> docs/formatdomain.rst | 5 +
> docs/schemas/domaincommon.rng | 5 +
> src/conf/domain_conf.c| 12
> src/conf/domain_conf.h| 1 +
> 4 files changed, 23 insertions(+)
[...]
A XML2XML test case would show that 'no' is useless in this impl, see
below.
> diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h
> index 930eed60de..544ec1b2fa 100644
> --- a/src/conf/domain_conf.h
> +++ b/src/conf/domain_conf.h
> @@ -1712,6 +1712,7 @@ struct _virDomainGraphicsDef {
> char *keymap;
> virDomainGraphicsAuthDef auth;
> int sharePolicy;
> +bool powerControl;
This is declared as bool.
> } vnc;
> struct {
> char *display;
> diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
> index b731744f04..91933bf292 100644
> --- a/src/conf/domain_conf.c
> +++ b/src/conf/domain_conf.c
[...]
> @@ -13206,6 +13207,13 @@
> virDomainGraphicsDefParseXMLVNC(virDomainGraphicsDefPtr def,
> }
> }
>
> +if (powerControl &&
> +virStringParseYesNo(powerControl, &def->data.vnc.powerControl) < 0) {
> +virReportError(VIR_ERR_INTERNAL_ERROR,
> + _("cannot parse vnc power control '%s'"),
> powerControl);
> +return -1;
> +}
> +
> def->data.vnc.keymap = virXMLPropString(node, "keymap");
>
> if (virDomainGraphicsAuthDefParseXML(node, &def->data.vnc.auth,
> @@ -27148,6 +27156,10 @@ virDomainGraphicsDefFormat(virBufferPtr buf,
>virDomainGraphicsVNCSharePolicyTypeToString(
>def->data.vnc.sharePolicy));
>
> +if (def->data.vnc.powerControl)
> +virBufferAsprintf(buf, " powerControl='%s'",
> + def->data.vnc.powerControl ? "yes" : "no");
So this doesn't make much sense. You can't use 'no' since it will vanish
from the XML.
Did you want to use a Tristate?
> +
> virDomainGraphicsAuthDefFormatAttr(buf, &def->data.vnc.auth, flags);
> break;
>
> --
> 2.29.2
>
[libvirt PATCH] docs: formatdomain: fix link to memoryBacking element
Fixes: e88bdaf789b6f1cc5347b217240f15afd86a94c1 Signed-off-by: Ján Tomko --- Pushed as trivial. docs/formatdomain.rst | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/formatdomain.rst b/docs/formatdomain.rst index eafd6b3396..2587106191 100644 --- a/docs/formatdomain.rst +++ b/docs/formatdomain.rst @@ -2615,7 +2615,7 @@ paravirtualized driver is specified via the ``disk`` element. ``vhostuser`` Enables the hypervisor to connect to another process using vhost-user protocol. Requires shared memory configured for the VM, for more details - see ``access`` mode for `memoryBacking <#elementsMemoryBacking>` element. + see ``access`` mode for `memoryBacking <#elementsMemoryBacking>`__ element. The ``source`` element has following mandatory attributes: -- 2.29.2
[libvirt PATCH 3/3] qemu: wire up support for VNC power control options
This allows the VNC client user to perform a shutdown, reboot and reset
of the VM from the host side.
Signed-off-by: Daniel P. Berrangé
---
src/qemu/qemu_command.c | 9 +
tests/qemuxml2argvdata/graphics-vnc-policy.args | 2 +-
tests/qemuxml2argvdata/graphics-vnc-policy.xml | 2 +-
tests/qemuxml2argvtest.c| 2 +-
4 files changed, 12 insertions(+), 3 deletions(-)
diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
index d801018aa2..266cf7332e 100644
--- a/src/qemu/qemu_command.c
+++ b/src/qemu/qemu_command.c
@@ -7700,6 +7700,15 @@ qemuBuildGraphicsVNCCommandLine(virQEMUDriverConfigPtr
cfg,
/* TODO: Support ACLs later */
}
+if (graphics->data.vnc.powerControl) {
+if (!virQEMUCapsGet(qemuCaps, QEMU_CAPS_VNC_POWER_CONTROL)) {
+virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
+ _("VNC power control is not available"));
+return -1;
+}
+virBufferAddLit(&opt, ",power-control=on");
+}
+
virCommandAddArg(cmd, "-vnc");
virCommandAddArgBuffer(cmd, &opt);
if (graphics->data.vnc.keymap)
diff --git a/tests/qemuxml2argvdata/graphics-vnc-policy.args
b/tests/qemuxml2argvdata/graphics-vnc-policy.args
index 85b7d722e0..d3a8d66161 100644
--- a/tests/qemuxml2argvdata/graphics-vnc-policy.args
+++ b/tests/qemuxml2argvdata/graphics-vnc-policy.args
@@ -26,5 +26,5 @@ server=on,wait=off \
-usb \
-drive file=/dev/HostVG/QEMUGuest1,format=raw,if=none,id=drive-ide0-0-0 \
-device ide-hd,bus=ide.0,unit=0,drive=drive-ide0-0-0,id=ide0-0-0,bootindex=1 \
--vnc '[::]:59630,share=allow-exclusive' \
+-vnc '[::]:59630,share=allow-exclusive,power-control=on' \
-vga cirrus
diff --git a/tests/qemuxml2argvdata/graphics-vnc-policy.xml
b/tests/qemuxml2argvdata/graphics-vnc-policy.xml
index 344f019079..4bee773438 100644
--- a/tests/qemuxml2argvdata/graphics-vnc-policy.xml
+++ b/tests/qemuxml2argvdata/graphics-vnc-policy.xml
@@ -25,7 +25,7 @@
-
+
diff --git a/tests/qemuxml2argvtest.c b/tests/qemuxml2argvtest.c
index db438c5466..b4df042fea 100644
--- a/tests/qemuxml2argvtest.c
+++ b/tests/qemuxml2argvtest.c
@@ -1364,7 +1364,7 @@ mymain(void)
QEMU_CAPS_VNC,
QEMU_CAPS_DEVICE_CIRRUS_VGA);
DO_TEST("graphics-vnc-policy", QEMU_CAPS_VNC,
-QEMU_CAPS_DEVICE_CIRRUS_VGA);
+QEMU_CAPS_DEVICE_CIRRUS_VGA, QEMU_CAPS_VNC_POWER_CONTROL);
DO_TEST("graphics-vnc-no-listen-attr", QEMU_CAPS_VNC,
QEMU_CAPS_DEVICE_CIRRUS_VGA);
DO_TEST("graphics-vnc-remove-generated-socket", QEMU_CAPS_VNC,
--
2.29.2
[libvirt PATCH 0/3] qemu: add support for VNC power control option
Daniel P. Berrangé (3): conf: add support for VNC power control setting qemu: probe for -vnc power-control option support qemu: wire up support for VNC power control options docs/formatdomain.rst| 5 + docs/schemas/domaincommon.rng| 5 + src/conf/domain_conf.c | 12 src/conf/domain_conf.h | 1 + src/qemu/qemu_capabilities.c | 2 ++ src/qemu/qemu_capabilities.h | 1 + src/qemu/qemu_command.c | 9 + tests/qemucapabilitiesdata/caps_6.0.0.x86_64.xml | 1 + tests/qemuxml2argvdata/graphics-vnc-policy.args | 2 +- tests/qemuxml2argvdata/graphics-vnc-policy.xml | 2 +- tests/qemuxml2argvtest.c | 2 +- 11 files changed, 39 insertions(+), 3 deletions(-) -- 2.29.2
[libvirt PATCH 2/3] qemu: probe for -vnc power-control option support
Signed-off-by: Daniel P. Berrangé
---
src/qemu/qemu_capabilities.c | 2 ++
src/qemu/qemu_capabilities.h | 1 +
tests/qemucapabilitiesdata/caps_6.0.0.x86_64.xml | 1 +
3 files changed, 4 insertions(+)
diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c
index 3f8593a9e5..8fa23f14be 100644
--- a/src/qemu/qemu_capabilities.c
+++ b/src/qemu/qemu_capabilities.c
@@ -617,6 +617,7 @@ VIR_ENUM_IMPL(virQEMUCaps,
"cpu-max",
"memory-backend-file.x-use-canonical-path-for-ramblock-id",
"vnc-opts",
+ "vnc-power-control",
);
@@ -3297,6 +3298,7 @@ static struct virQEMUCapsCommandLineProps
virQEMUCapsCommandLine[] = {
{ "fw_cfg", "file", QEMU_CAPS_FW_CFG },
{ "fsdev", "fmode", QEMU_CAPS_FSDEV_CREATEMODE }, /* Could have also
checked fsdev->dmode */
{ "vnc", "display", QEMU_CAPS_VNC_OPTS },
+{ "vnc", "power-control", QEMU_CAPS_VNC_POWER_CONTROL },
};
static int
diff --git a/src/qemu/qemu_capabilities.h b/src/qemu/qemu_capabilities.h
index 38574eef16..e327db0148 100644
--- a/src/qemu/qemu_capabilities.h
+++ b/src/qemu/qemu_capabilities.h
@@ -597,6 +597,7 @@ typedef enum { /* virQEMUCapsFlags grouping marker for
syntax-check */
QEMU_CAPS_CPU_MAX, /* -cpu max */
QEMU_CAPS_X_USE_CANONICAL_PATH_FOR_RAMBLOCK_ID, /* -object
memory-backend-file,x-use-canonical-path-for-ramblock-id= */
QEMU_CAPS_VNC_OPTS, /* -vnc uses QemuOpts parser instead of custom code */
+QEMU_CAPS_VNC_POWER_CONTROL, /* -vnc power-control option */
QEMU_CAPS_LAST /* this must always be the last item */
} virQEMUCapsFlags;
diff --git a/tests/qemucapabilitiesdata/caps_6.0.0.x86_64.xml
b/tests/qemucapabilitiesdata/caps_6.0.0.x86_64.xml
index 23fb5b7393..3ef05ec94e 100644
--- a/tests/qemucapabilitiesdata/caps_6.0.0.x86_64.xml
+++ b/tests/qemucapabilitiesdata/caps_6.0.0.x86_64.xml
@@ -256,6 +256,7 @@
+
5002050
0
43100242
--
2.29.2
[libvirt PATCH 1/3] conf: add support for VNC power control setting
The option instructs the
VNC server to enable an extension that lets the client perform a
graceful shutdown, reboot and hard reset.
This is enabled by default since it cannot be assumed that the VNC
client user has administrator rights over the guest OS. In the case
where the VNC user is a guest administrator though, it is reasonable
to allow direct power control host side too.
Signed-off-by: Daniel P. Berrangé
---
docs/formatdomain.rst | 5 +
docs/schemas/domaincommon.rng | 5 +
src/conf/domain_conf.c| 12
src/conf/domain_conf.h| 1 +
4 files changed, 23 insertions(+)
diff --git a/docs/formatdomain.rst b/docs/formatdomain.rst
index eafd6b3396..580319365c 100644
--- a/docs/formatdomain.rst
+++ b/docs/formatdomain.rst
@@ -5791,6 +5791,11 @@ interaction with the admin.
``autoport`` having no effect due to security reasons) :since:`Since
1.0.6` .
+ For VNC, the ``powerControl`` attribute can be used to enable VM
shutdown,
+ reboot and reset power control features for the VNC client. This is
+ appropriate if the authenticated VNC client user already has
administrator
+ privileges in the guest :since:`Since 7.1.0`.
+
Although VNC doesn't support OpenGL natively, it can be paired with
graphics type ``egl-headless`` (see below) which will instruct QEMU to
open and use drm nodes for OpenGL rendering.
diff --git a/docs/schemas/domaincommon.rng b/docs/schemas/domaincommon.rng
index e6de934456..49dc4b5130 100644
--- a/docs/schemas/domaincommon.rng
+++ b/docs/schemas/domaincommon.rng
@@ -3663,6 +3663,11 @@
+
+
+
+
+
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index b731744f04..91933bf292 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -13150,6 +13150,7 @@ virDomainGraphicsDefParseXMLVNC(virDomainGraphicsDefPtr
def,
g_autofree char *websocketGenerated = virXMLPropString(node,
"websocketGenerated");
g_autofree char *sharePolicy = virXMLPropString(node, "sharePolicy");
g_autofree char *autoport = virXMLPropString(node, "autoport");
+g_autofree char *powerControl = virXMLPropString(node, "powerControl");
if (virDomainGraphicsListensParseXML(def, node, ctxt, flags) < 0)
return -1;
@@ -13206,6 +13207,13 @@
virDomainGraphicsDefParseXMLVNC(virDomainGraphicsDefPtr def,
}
}
+if (powerControl &&
+virStringParseYesNo(powerControl, &def->data.vnc.powerControl) < 0) {
+virReportError(VIR_ERR_INTERNAL_ERROR,
+ _("cannot parse vnc power control '%s'"), powerControl);
+return -1;
+}
+
def->data.vnc.keymap = virXMLPropString(node, "keymap");
if (virDomainGraphicsAuthDefParseXML(node, &def->data.vnc.auth,
@@ -27148,6 +27156,10 @@ virDomainGraphicsDefFormat(virBufferPtr buf,
virDomainGraphicsVNCSharePolicyTypeToString(
def->data.vnc.sharePolicy));
+if (def->data.vnc.powerControl)
+virBufferAsprintf(buf, " powerControl='%s'",
+ def->data.vnc.powerControl ? "yes" : "no");
+
virDomainGraphicsAuthDefFormatAttr(buf, &def->data.vnc.auth, flags);
break;
diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h
index 930eed60de..544ec1b2fa 100644
--- a/src/conf/domain_conf.h
+++ b/src/conf/domain_conf.h
@@ -1712,6 +1712,7 @@ struct _virDomainGraphicsDef {
char *keymap;
virDomainGraphicsAuthDef auth;
int sharePolicy;
+bool powerControl;
} vnc;
struct {
char *display;
--
2.29.2
Re: [libvirt PATCH 3/4] ci: list-images: Split some generic logic to a util module
On Wed, 2021-02-10 at 18:24 +0100, Erik Skultety wrote: > +++ b/ci/list-images.py > @@ -1,29 +1,32 @@ > #!/usr/bin/env python3 > > -import json > -import urllib.request as urllib > +import containers.util as util As we add more Python code here, I think it makes sense to have a single util module in the top-level ci/ directory instead of one for the ci/containers/ directory and potentially another one for the ci/cirrus directory. -- Andrea Bolognani / Red Hat / Virtualization
Re: [libvirt PATCH 2/4] ci: Rewrite list-images from Bash to Python
On Wed, 2021-02-10 at 18:24 +0100, Erik Skultety wrote:
> +# skip the "ci-" prefix each of our container images' name has
> +names = [i["name"][3:] for i in data]
I would prefer something like
PREFIX = "ci-"
names = [i["name"][len(PREFIX):] for i in data]
here, rather than hardcoding the length of the string.
> +names_native = [name for name in names if "cross" not in name]
> +names_cross = [name for name in names if "cross" in name]
> +
> +print("Available x86 container images:\n")
> +print("\t" + "\n\t".join(names_native))
Please use two or four spaces instead of tabs.
More high-level feedback about the patch:
* you should drop the sh implementation at the same time as you're
adding the python one;
* adding this standalone script in one patch only to refactor most
of its code away to a separate module in the next one leads to
unnecessary churn and more work for the reviewer: please just add
the script in its intended form in the first place.
--
Andrea Bolognani / Red Hat / Virtualization
Re: [libvirt PATCH 2/5] qemu: use on|off for -vnc boolean option values
On a Tuesday in 2021, Daniel P. Berrangé wrote: The preferred syntax for boolean options is to set the value "on" or "off". QEMU 7.1.0 will deprecate the short format we currently use. The long format has been supported with -vnc since the change to use QemuOpts in 2.2.0, so we check based on the new capability flag. Signed-off-by: Daniel P. Berrangé --- src/qemu/qemu_command.c| 18 ++ .../graphics-vnc-tls-secret.x86_64-latest.args | 2 +- .../graphics-vnc-tls.x86_64-2.4.0.args | 2 +- .../graphics-vnc-tls.x86_64-latest.args| 2 +- 4 files changed, 17 insertions(+), 7 deletions(-) Reviewed-by: Ján Tomko Jano signature.asc Description: PGP signature
Re: [libvirt PATCH 1/5] qemu: probe for -vnc supporting use of QemuOpts syntax
On a Tuesday in 2021, Daniel P. Berrangé wrote: This was introduced in QEMU 2.2.0, and is visible by -vnc appearing in the "query-command-line-options" data. Signed-off-by: Daniel P. Berrangé --- src/qemu/qemu_capabilities.c | 2 ++ src/qemu/qemu_capabilities.h | 1 + tests/qemucapabilitiesdata/caps_2.10.0.aarch64.xml | 1 + [...] tests/qemucapabilitiesdata/caps_5.2.0.x86_64.xml | 1 + tests/qemucapabilitiesdata/caps_6.0.0.x86_64.xml | 1 + 54 files changed, 55 insertions(+) Reviewed-by: Ján Tomko Jano signature.asc Description: PGP signature
Re: [libvirt PATCH 0/5] qemu: more cleanup of boolean option handling
On Tue, Feb 16, 2021 at 12:57:00 +, Daniel Berrange wrote: > This completes the conversion to standarize on on|off for all boolean > options in QEMU. > > Daniel P. Berrang=C3=A9 (5): > qemu: probe for -vnc supporting use of QemuOpts syntax > qemu: use on|off for -vnc boolean option values > qemu: use on|off instead of yes|no for -object boolean properties > qemu: use on|off instead of yes|no for -drive boolean properties > qemu: remove support for generating yes|no boolean options Series: Reviewed-by: Peter Krempa if you don't expand output files unnecessarily in 3/5
Re: [libvirt PATCH 1/4] ci: Makefile: Specify a help target to replace ci-help
On Wed, 2021-02-10 at 18:24 +0100, Erik Skultety wrote: > It's quite pointless to have a 'ci-help' target in the Makefile when > one needs to actually open the Makefile to go through the list of > targets to know what functionality the Makefile actually provides. It's > much more intuitive to run "make help" in that case. Therefore, add a > 'help' target and replace the old 'ci-help' target with it. > > Signed-off-by: Erik Skultety > --- > ci/Makefile | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) Reviewed-by: Andrea Bolognani -- Andrea Bolognani / Red Hat / Virtualization
Re: [libvirt PATCH 3/5] qemu: use on|off instead of yes|no for -object boolean properties
On Tue, Feb 16, 2021 at 13:26:38 +, Daniel Berrange wrote: > On Tue, Feb 16, 2021 at 02:21:56PM +0100, Peter Krempa wrote: > > On Tue, Feb 16, 2021 at 12:57:03 +, Daniel Berrange wrote: > > > QEMU has long accepted many different values for boolean properties, but > > > set accepted has been different depending on which QEMU parser you hit. > > > > > > The on|off values were supported by all QEMU parsers. The yes|no, y|n, > > > true|false values were only partially supported: > > > > > > https://lists.gnu.org/archive/html/qemu-devel/2020-11/msg01012.html > > > > > > Thus we should standardize on on|off everywhere since that is most > > > widely supported in QEMU. > > > > > > Signed-off-by: Daniel P. Berrangé > > > --- > > > > [...] > > > > > mode change 12 => 100644 > > > tests/qemuxml2argvdata/aarch64-gic-default-both.args > > > mode change 12 => 100644 > > > tests/qemuxml2argvdata/aarch64-gic-default-v2.args > > > mode change 12 => 100644 > > > tests/qemuxml2argvdata/aarch64-gic-default-v3.args > > > mode change 12 => 100644 > > > tests/qemuxml2argvdata/aarch64-gic-default.args > > > mode change 12 => 100644 > > > tests/qemuxml2argvdata/aarch64-gic-none-both.args > > > mode change 12 => 100644 > > > tests/qemuxml2argvdata/aarch64-gic-none-v2.args > > > mode change 12 => 100644 > > > tests/qemuxml2argvdata/aarch64-gic-none-v3.args > > > mode change 12 => 100644 tests/qemuxml2argvdata/aarch64-gic-none.args > > > mode change 12 => 100644 tests/qemuxml2argvdata/cpu-check-full.args > > > mode change 12 => 100644 > > > tests/qemuxml2argvdata/cpu-check-partial.args > > > mode change 12 => 100644 > > > tests/qemuxml2argvdata/disk-backing-chains-index.x86_64-2.12.0.args > > > mode change 12 => 100644 > > > tests/qemuxml2argvdata/disk-backing-chains-index.x86_64-latest.args > > > mode change 12 => 100644 > > > tests/qemuxml2argvdata/mach-virt-console-native.args > > > mode change 12 => 100644 > > > tests/qemuxml2argvdata/mach-virt-serial+console-native.args > > > mode change 12 => 100644 > > > tests/qemuxml2argvdata/mach-virt-serial-compat.args > > > mode change 12 => 100644 > > > tests/qemuxml2argvdata/pci-rom-disabled-invalid.args > > > mode change 12 => 100644 > > > tests/qemuxml2argvdata/pseries-console-native.args > > > mode change 12 => 100644 > > > tests/qemuxml2argvdata/pseries-serial+console-native.args > > > mode change 12 => 100644 > > > tests/qemuxml2argvdata/pseries-serial-compat.args > > > mode change 12 => 100644 tests/qemuxml2argvdata/user-aliases2.args > > > > Any particular reason why these are converted from symlink to full file? > > Urgh, yet again. I hate these symlinks :=( > > It happens whenever you do a in-place search-replace with sed/perl/etc Yup. in this case VIR_TEST_REGENERATE_OUTPUT=1 does the right thing actually.
Re: [libvirt PATCH 3/5] qemu: use on|off instead of yes|no for -object boolean properties
On Tue, Feb 16, 2021 at 02:21:56PM +0100, Peter Krempa wrote: > On Tue, Feb 16, 2021 at 12:57:03 +, Daniel Berrange wrote: > > QEMU has long accepted many different values for boolean properties, but > > set accepted has been different depending on which QEMU parser you hit. > > > > The on|off values were supported by all QEMU parsers. The yes|no, y|n, > > true|false values were only partially supported: > > > > https://lists.gnu.org/archive/html/qemu-devel/2020-11/msg01012.html > > > > Thus we should standardize on on|off everywhere since that is most > > widely supported in QEMU. > > > > Signed-off-by: Daniel P. Berrangé > > --- > > [...] > > > mode change 12 => 100644 > > tests/qemuxml2argvdata/aarch64-gic-default-both.args > > mode change 12 => 100644 > > tests/qemuxml2argvdata/aarch64-gic-default-v2.args > > mode change 12 => 100644 > > tests/qemuxml2argvdata/aarch64-gic-default-v3.args > > mode change 12 => 100644 > > tests/qemuxml2argvdata/aarch64-gic-default.args > > mode change 12 => 100644 > > tests/qemuxml2argvdata/aarch64-gic-none-both.args > > mode change 12 => 100644 > > tests/qemuxml2argvdata/aarch64-gic-none-v2.args > > mode change 12 => 100644 > > tests/qemuxml2argvdata/aarch64-gic-none-v3.args > > mode change 12 => 100644 tests/qemuxml2argvdata/aarch64-gic-none.args > > mode change 12 => 100644 tests/qemuxml2argvdata/cpu-check-full.args > > mode change 12 => 100644 tests/qemuxml2argvdata/cpu-check-partial.args > > mode change 12 => 100644 > > tests/qemuxml2argvdata/disk-backing-chains-index.x86_64-2.12.0.args > > mode change 12 => 100644 > > tests/qemuxml2argvdata/disk-backing-chains-index.x86_64-latest.args > > mode change 12 => 100644 > > tests/qemuxml2argvdata/mach-virt-console-native.args > > mode change 12 => 100644 > > tests/qemuxml2argvdata/mach-virt-serial+console-native.args > > mode change 12 => 100644 > > tests/qemuxml2argvdata/mach-virt-serial-compat.args > > mode change 12 => 100644 > > tests/qemuxml2argvdata/pci-rom-disabled-invalid.args > > mode change 12 => 100644 > > tests/qemuxml2argvdata/pseries-console-native.args > > mode change 12 => 100644 > > tests/qemuxml2argvdata/pseries-serial+console-native.args > > mode change 12 => 100644 > > tests/qemuxml2argvdata/pseries-serial-compat.args > > mode change 12 => 100644 tests/qemuxml2argvdata/user-aliases2.args > > Any particular reason why these are converted from symlink to full file? Urgh, yet again. I hate these symlinks :=( It happens whenever you do a in-place search-replace with sed/perl/etc 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/5] qemu: use on|off instead of yes|no for -object boolean properties
On Tue, Feb 16, 2021 at 12:57:03 +, Daniel Berrange wrote: > QEMU has long accepted many different values for boolean properties, but > set accepted has been different depending on which QEMU parser you hit. > > The on|off values were supported by all QEMU parsers. The yes|no, y|n, > true|false values were only partially supported: > > https://lists.gnu.org/archive/html/qemu-devel/2020-11/msg01012.html > > Thus we should standardize on on|off everywhere since that is most > widely supported in QEMU. > > Signed-off-by: Daniel P. Berrangé > --- [...] > mode change 12 => 100644 > tests/qemuxml2argvdata/aarch64-gic-default-both.args > mode change 12 => 100644 > tests/qemuxml2argvdata/aarch64-gic-default-v2.args > mode change 12 => 100644 > tests/qemuxml2argvdata/aarch64-gic-default-v3.args > mode change 12 => 100644 tests/qemuxml2argvdata/aarch64-gic-default.args > mode change 12 => 100644 > tests/qemuxml2argvdata/aarch64-gic-none-both.args > mode change 12 => 100644 tests/qemuxml2argvdata/aarch64-gic-none-v2.args > mode change 12 => 100644 tests/qemuxml2argvdata/aarch64-gic-none-v3.args > mode change 12 => 100644 tests/qemuxml2argvdata/aarch64-gic-none.args > mode change 12 => 100644 tests/qemuxml2argvdata/cpu-check-full.args > mode change 12 => 100644 tests/qemuxml2argvdata/cpu-check-partial.args > mode change 12 => 100644 > tests/qemuxml2argvdata/disk-backing-chains-index.x86_64-2.12.0.args > mode change 12 => 100644 > tests/qemuxml2argvdata/disk-backing-chains-index.x86_64-latest.args > mode change 12 => 100644 > tests/qemuxml2argvdata/mach-virt-console-native.args > mode change 12 => 100644 > tests/qemuxml2argvdata/mach-virt-serial+console-native.args > mode change 12 => 100644 > tests/qemuxml2argvdata/mach-virt-serial-compat.args > mode change 12 => 100644 > tests/qemuxml2argvdata/pci-rom-disabled-invalid.args > mode change 12 => 100644 > tests/qemuxml2argvdata/pseries-console-native.args > mode change 12 => 100644 > tests/qemuxml2argvdata/pseries-serial+console-native.args > mode change 12 => 100644 > tests/qemuxml2argvdata/pseries-serial-compat.args > mode change 12 => 100644 tests/qemuxml2argvdata/user-aliases2.args Any particular reason why these are converted from symlink to full file?
Re: [PATCH RESEND 00/20] handle missing SR-IOV VF hostdevs during running domains
Ping for reviews (patches 1-4 and 7-8 already pushed). On 1/18/21 4:53 PM, Daniel Henrique Barboza wrote: This is the rebased version of https://www.redhat.com/archives/libvir-list/2021-January/msg00028.html with master at 57b1ddcaaa5b5. No changes made aside from trivial conflicts fixes. I also removed the military jargon from the previous subject to make it clear what this series is doing. Daniel Henrique Barboza (20): virpci, domain_audit: use virPCIDeviceAddressAsString() qemu, lxc: move NodeDeviceGetPCIInfo() function to domain_driver.c domain_driver.c: use PCI address with virDomainDriverNodeDeviceGetPCIInfo() virpci.c: simplify virPCIDeviceNew() signature virpci: introduce virPCIDeviceExists() virhostdev.c: virHostdevGetPCIHostDevice() now reports missing device security_selinux.c: modernize set/restore hostdev subsys label functions security_dac.c: modernize hostdev label set/restore functions dac, selinux: skip setting/restoring label for absent PCI devices libvirt-nodedev.c: remove return value from virNodeDeviceFree() qemu_driver.c: modernize qemuNodeDeviceReAttach() libxl_driver.c: modernize libxlNodeDeviceReAttach() virsh-domain.c: modernize cmdDetachDevice() virhostdev.c: add virHostdevIsPCIDevice() helper qemu_cgroup.c: skip absent PCI devices in qemuTeardownHostdevCgroup() virpci.c: use virPCIDeviceAddressPtr in virPCIDeviceListFindIndex() virpci.c: use virPCIDeviceAddressPtr in virPCIDeviceListFind() virpci.c: use virPCIDeviceAddressPtr in virPCIDeviceListSteal() virpci.c: use virPCIDeviceAddressPtr in virPCIDeviceListDel() virhostdev.c: remove missing PCI devs from hostdev manager include/libvirt/libvirt-nodedev.h | 2 +- src/conf/domain_audit.c| 6 +- src/datatypes.h| 2 + src/hypervisor/domain_driver.c | 30 ++ src/hypervisor/domain_driver.h | 4 ++ src/hypervisor/virhostdev.c| 88 ++-- src/hypervisor/virhostdev.h| 2 + src/libvirt-nodedev.c | 15 +++-- src/libvirt_private.syms | 3 + src/libxl/libxl_driver.c | 87 src/node_device/node_device_udev.c | 11 ++-- src/qemu/qemu_cgroup.c | 10 src/qemu/qemu_domain_address.c | 5 +- src/qemu/qemu_driver.c | 81 +++--- src/security/security_apparmor.c | 3 +- src/security/security_dac.c| 61 src/security/security_selinux.c| 66 + src/security/virt-aa-helper.c | 6 +- src/util/virnetdev.c | 3 +- src/util/virnvme.c | 5 +- src/util/virpci.c | 93 ++ src/util/virpci.h | 14 ++--- tests/virhostdevtest.c | 3 +- tests/virpcitest.c | 35 --- tools/virsh-domain.c | 18 ++ 25 files changed, 325 insertions(+), 328 deletions(-)
Re: [libvirt PATCH 0/2] ci: Tiny changes to the Makefile
On Mon, 2021-02-15 at 13:11 +0100, Erik Skultety wrote: > > Erik Skultety (2): > ci: Drop the CI_PREPARE_SCRIPT variable > ci: Makefile: Expose CI_IMAGE_PREFIX and CI_IMAGE_TAG in 'ci-help' > > ci/Makefile | 17 - > 1 file changed, 8 insertions(+), 9 deletions(-) Reviewed-by: Andrea Bolognani -- Andrea Bolognani / Red Hat / Virtualization
[libvirt PATCH 5/5] qemu: remove support for generating yes|no boolean options
All callers are now using the on|off syntax, so yes|no is a unreachable
code path.
Signed-off-by: Daniel P. Berrangé
---
src/util/virqemu.c | 50 +
src/util/virqemu.h | 10 +++-
tests/qemucommandutiltest.c | 10
3 files changed, 25 insertions(+), 45 deletions(-)
diff --git a/src/util/virqemu.c b/src/util/virqemu.c
index 5b0dc25bc1..57ee42dd16 100644
--- a/src/util/virqemu.c
+++ b/src/util/virqemu.c
@@ -37,7 +37,6 @@ struct virQEMUCommandLineJSONIteratorData {
const char *prefix;
virBufferPtr buf;
const char *skipKey;
-bool onOff;
virQEMUBuildCommandLineJSONArrayFormatFunc arrayFunc;
};
@@ -47,7 +46,6 @@ virQEMUBuildCommandLineJSONRecurse(const char *key,
virJSONValuePtr value,
virBufferPtr buf,
const char *skipKey,
- bool onOff,
virQEMUBuildCommandLineJSONArrayFormatFunc
arrayFunc,
bool nested);
@@ -57,8 +55,7 @@ int
virQEMUBuildCommandLineJSONArrayBitmap(const char *key,
virJSONValuePtr array,
virBufferPtr buf,
- const char *skipKey G_GNUC_UNUSED,
- bool onOff G_GNUC_UNUSED)
+ const char *skipKey G_GNUC_UNUSED)
{
ssize_t pos = -1;
ssize_t end;
@@ -87,8 +84,7 @@ int
virQEMUBuildCommandLineJSONArrayNumbered(const char *key,
virJSONValuePtr array,
virBufferPtr buf,
- const char *skipKey,
- bool onOff)
+ const char *skipKey)
{
virJSONValuePtr member;
size_t i;
@@ -99,7 +95,7 @@ virQEMUBuildCommandLineJSONArrayNumbered(const char *key,
member = virJSONValueArrayGet((virJSONValuePtr) array, i);
prefix = g_strdup_printf("%s.%zu", key, i);
-if (virQEMUBuildCommandLineJSONRecurse(prefix, member, buf, skipKey,
onOff,
+if (virQEMUBuildCommandLineJSONRecurse(prefix, member, buf, skipKey,
virQEMUBuildCommandLineJSONArrayNumbered,
true) < 0)
return 0;
@@ -125,8 +121,7 @@ static int
virQEMUBuildCommandLineJSONArrayObjectsStr(const char *key,
virJSONValuePtr array,
virBufferPtr buf,
- const char *skipKey G_GNUC_UNUSED,
- bool onOff G_GNUC_UNUSED)
+ const char *skipKey G_GNUC_UNUSED)
{
g_auto(virBuffer) tmp = VIR_BUFFER_INITIALIZER;
size_t i;
@@ -163,11 +158,11 @@ virQEMUBuildCommandLineJSONIterate(const char *key,
tmpkey = g_strdup_printf("%s.%s", data->prefix, key);
return virQEMUBuildCommandLineJSONRecurse(tmpkey, value, data->buf,
- data->skipKey, data->onOff,
+ data->skipKey,
data->arrayFunc, false);
} else {
return virQEMUBuildCommandLineJSONRecurse(key, value, data->buf,
- data->skipKey, data->onOff,
+ data->skipKey,
data->arrayFunc, false);
}
}
@@ -178,11 +173,10 @@ virQEMUBuildCommandLineJSONRecurse(const char *key,
virJSONValuePtr value,
virBufferPtr buf,
const char *skipKey,
- bool onOff,
virQEMUBuildCommandLineJSONArrayFormatFunc
arrayFunc,
bool nested)
{
-struct virQEMUCommandLineJSONIteratorData data = { key, buf, skipKey,
onOff, arrayFunc };
+struct virQEMUCommandLineJSONIteratorData data = { key, buf, skipKey,
arrayFunc };
virJSONType type = virJSONValueGetType(value);
virJSONValuePtr elem;
bool tmp;
@@ -207,18 +201,10 @@ virQEMUBuildCommandLineJSONRecurse(const char *key,
case VIR_JSON_TYPE_BOOLEAN:
virJSONValueGetBoolean(value, &tmp);
-if (onOff) {
-if (tmp)
-virBufferAsprintf(buf, "%s=on,", key);
-else
-virBufferAsprintf(buf, "%s=off,", key);
-} else {
-if (tmp)
-virBufferAsprintf(buf, "%s=yes,", key);
-else
-
[libvirt PATCH 4/5] qemu: use on|off instead of yes|no for -drive boolean properties
QEMU has long accepted many different values for boolean properties, but
set accepted has been different depending on which QEMU parser you hit.
The on|off values were supported by all QEMU parsers. The yes|no, y|n,
true|false values were only partially supported:
https://lists.gnu.org/archive/html/qemu-devel/2020-11/msg01012.html
Thus we should standardize on on|off everywhere since that is most
widely supported in QEMU.
Signed-off-by: Daniel P. Berrangé
---
src/util/virqemu.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/src/util/virqemu.c b/src/util/virqemu.c
index 4136012c70..5b0dc25bc1 100644
--- a/src/util/virqemu.c
+++ b/src/util/virqemu.c
@@ -362,7 +362,7 @@ virQEMUBuildDriveCommandlineFromJSON(virJSONValuePtr srcdef)
{
g_auto(virBuffer) buf = VIR_BUFFER_INITIALIZER;
-if (virQEMUBuildCommandLineJSON(srcdef, &buf, NULL, false,
+if (virQEMUBuildCommandLineJSON(srcdef, &buf, NULL, true,
virQEMUBuildCommandLineJSONArrayNumbered)
< 0)
return NULL;
--
2.29.2
[libvirt PATCH 2/5] qemu: use on|off for -vnc boolean option values
The preferred syntax for boolean options is to set the value "on" or
"off". QEMU 7.1.0 will deprecate the short format we currently use.
The long format has been supported with -vnc since the change to use
QemuOpts in 2.2.0, so we check based on the new capability flag.
Signed-off-by: Daniel P. Berrangé
---
src/qemu/qemu_command.c| 18 ++
.../graphics-vnc-tls-secret.x86_64-latest.args | 2 +-
.../graphics-vnc-tls.x86_64-2.4.0.args | 2 +-
.../graphics-vnc-tls.x86_64-latest.args| 2 +-
4 files changed, 17 insertions(+), 7 deletions(-)
diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
index a8342ff7d5..d801018aa2 100644
--- a/src/qemu/qemu_command.c
+++ b/src/qemu/qemu_command.c
@@ -7644,8 +7644,12 @@ qemuBuildGraphicsVNCCommandLine(virQEMUDriverConfigPtr
cfg,
graphics->data.vnc.sharePolicy));
}
-if (graphics->data.vnc.auth.passwd || cfg->vncPassword)
-virBufferAddLit(&opt, ",password");
+if (graphics->data.vnc.auth.passwd || cfg->vncPassword) {
+if (virQEMUCapsGet(qemuCaps, QEMU_CAPS_VNC_OPTS))
+virBufferAddLit(&opt, ",password=on");
+else
+virBufferAddLit(&opt, ",password");
+}
if (cfg->vncTLS) {
qemuDomainGraphicsPrivatePtr gfxPriv =
QEMU_DOMAIN_GRAPHICS_PRIVATE(graphics);
@@ -7670,7 +7674,10 @@ qemuBuildGraphicsVNCCommandLine(virQEMUDriverConfigPtr
cfg,
virBufferAsprintf(&opt, ",tls-creds=%s", gfxPriv->tlsAlias);
} else {
-virBufferAddLit(&opt, ",tls");
+if (virQEMUCapsGet(qemuCaps, QEMU_CAPS_VNC_OPTS))
+virBufferAddLit(&opt, ",tls=on");
+else
+virBufferAddLit(&opt, ",tls");
if (cfg->vncTLSx509verify) {
virBufferAddLit(&opt, ",x509verify=");
virQEMUBuildBufferEscapeComma(&opt, cfg->vncTLSx509certdir);
@@ -7682,7 +7689,10 @@ qemuBuildGraphicsVNCCommandLine(virQEMUDriverConfigPtr
cfg,
}
if (cfg->vncSASL) {
-virBufferAddLit(&opt, ",sasl");
+if (virQEMUCapsGet(qemuCaps, QEMU_CAPS_VNC_OPTS))
+virBufferAddLit(&opt, ",sasl=on");
+else
+virBufferAddLit(&opt, ",sasl");
if (cfg->vncSASLdir)
virCommandAddEnvPair(cmd, "SASL_CONF_PATH", cfg->vncSASLdir);
diff --git a/tests/qemuxml2argvdata/graphics-vnc-tls-secret.x86_64-latest.args
b/tests/qemuxml2argvdata/graphics-vnc-tls-secret.x86_64-latest.args
index 00119fcd3d..eb0df17eda 100644
--- a/tests/qemuxml2argvdata/graphics-vnc-tls-secret.x86_64-latest.args
+++ b/tests/qemuxml2argvdata/graphics-vnc-tls-secret.x86_64-latest.args
@@ -34,7 +34,7 @@
data=9eao5F8qtkGt+seB1HYivWIxbtwUu6MQtg1zpj/oDtUsPr1q8wBYM91uEHCn6j/1,\
keyid=masterKey0,iv=AAECAwQFBgcICQoLDA0ODw==,format=base64 \
-object tls-creds-x509,id=vnc-tls-creds0,dir=/etc/pki/libvirt-vnc,\
endpoint=server,verify-peer=yes,passwordid=vnc-tls-creds0-secret0 \
--vnc 127.0.0.1:3,tls-creds=vnc-tls-creds0,sasl \
+-vnc 127.0.0.1:3,tls-creds=vnc-tls-creds0,sasl=on \
-device cirrus-vga,id=video0,bus=pci.0,addr=0x2 \
-sandbox on,obsolete=deny,elevateprivileges=deny,spawn=deny,\
resourcecontrol=deny \
diff --git a/tests/qemuxml2argvdata/graphics-vnc-tls.x86_64-2.4.0.args
b/tests/qemuxml2argvdata/graphics-vnc-tls.x86_64-2.4.0.args
index 471ff0dc77..78a47f1d30 100644
--- a/tests/qemuxml2argvdata/graphics-vnc-tls.x86_64-2.4.0.args
+++ b/tests/qemuxml2argvdata/graphics-vnc-tls.x86_64-2.4.0.args
@@ -26,6 +26,6 @@ server=on,wait=off \
-no-acpi \
-boot strict=on \
-device piix3-usb-uhci,id=usb,bus=pci.0,addr=0x1.0x2 \
--vnc 127.0.0.1:3,tls,x509verify=/etc/pki/libvirt-vnc,sasl \
+-vnc 127.0.0.1:3,tls=on,x509verify=/etc/pki/libvirt-vnc,sasl=on \
-device cirrus-vga,id=video0,bus=pci.0,addr=0x2 \
-msg timestamp=on
diff --git a/tests/qemuxml2argvdata/graphics-vnc-tls.x86_64-latest.args
b/tests/qemuxml2argvdata/graphics-vnc-tls.x86_64-latest.args
index b739a9b9c8..1c4b948b97 100644
--- a/tests/qemuxml2argvdata/graphics-vnc-tls.x86_64-latest.args
+++ b/tests/qemuxml2argvdata/graphics-vnc-tls.x86_64-latest.args
@@ -31,7 +31,7 @@ file=/tmp/lib/domain--1-QEMUGuest1/master-key.aes \
-device piix3-usb-uhci,id=usb,bus=pci.0,addr=0x1.0x2 \
-object tls-creds-x509,id=vnc-tls-creds0,dir=/etc/pki/libvirt-vnc,\
endpoint=server,verify-peer=yes \
--vnc 127.0.0.1:3,tls-creds=vnc-tls-creds0,sasl \
+-vnc 127.0.0.1:3,tls-creds=vnc-tls-creds0,sasl=on \
-device cirrus-vga,id=video0,bus=pci.0,addr=0x2 \
-sandbox on,obsolete=deny,elevateprivileges=deny,spawn=deny,\
resourcecontrol=deny \
--
2.29.2
[libvirt PATCH 0/5] qemu: more cleanup of boolean option handling
This completes the conversion to standarize on on|off for all boolean options in QEMU. Daniel P. Berrang=C3=A9 (5): qemu: probe for -vnc supporting use of QemuOpts syntax qemu: use on|off for -vnc boolean option values qemu: use on|off instead of yes|no for -object boolean properties qemu: use on|off instead of yes|no for -drive boolean properties qemu: remove support for generating yes|no boolean options src/qemu/qemu_capabilities.c | 2 + src/qemu/qemu_capabilities.h | 1 + src/qemu/qemu_command.c | 18 +- src/util/virqemu.c| 50 ++ src/util/virqemu.h| 10 +- .../caps_2.10.0.aarch64.xml | 1 + .../caps_2.10.0.ppc64.xml | 1 + .../caps_2.10.0.s390x.xml | 1 + .../caps_2.10.0.x86_64.xml| 1 + .../caps_2.11.0.s390x.xml | 1 + .../caps_2.11.0.x86_64.xml| 1 + .../caps_2.12.0.aarch64.xml | 1 + .../caps_2.12.0.ppc64.xml | 1 + .../caps_2.12.0.s390x.xml | 1 + .../caps_2.12.0.x86_64.xml| 1 + .../caps_2.4.0.x86_64.xml | 1 + .../caps_2.5.0.x86_64.xml | 1 + .../caps_2.6.0.aarch64.xml| 1 + .../qemucapabilitiesdata/caps_2.6.0.ppc64.xml | 1 + .../caps_2.6.0.x86_64.xml | 1 + .../qemucapabilitiesdata/caps_2.7.0.s390x.xml | 1 + .../caps_2.7.0.x86_64.xml | 1 + .../qemucapabilitiesdata/caps_2.8.0.s390x.xml | 1 + .../caps_2.8.0.x86_64.xml | 1 + .../qemucapabilitiesdata/caps_2.9.0.ppc64.xml | 1 + .../qemucapabilitiesdata/caps_2.9.0.s390x.xml | 1 + .../caps_2.9.0.x86_64.xml | 1 + .../qemucapabilitiesdata/caps_3.0.0.ppc64.xml | 1 + .../caps_3.0.0.riscv32.xml| 1 + .../caps_3.0.0.riscv64.xml| 1 + .../qemucapabilitiesdata/caps_3.0.0.s390x.xml | 1 + .../caps_3.0.0.x86_64.xml | 1 + .../qemucapabilitiesdata/caps_3.1.0.ppc64.xml | 1 + .../caps_3.1.0.x86_64.xml | 1 + .../caps_4.0.0.aarch64.xml| 1 + .../qemucapabilitiesdata/caps_4.0.0.ppc64.xml | 1 + .../caps_4.0.0.riscv32.xml| 1 + .../caps_4.0.0.riscv64.xml| 1 + .../qemucapabilitiesdata/caps_4.0.0.s390x.xml | 1 + .../caps_4.0.0.x86_64.xml | 1 + .../caps_4.1.0.x86_64.xml | 1 + .../caps_4.2.0.aarch64.xml| 1 + .../qemucapabilitiesdata/caps_4.2.0.ppc64.xml | 1 + .../qemucapabilitiesdata/caps_4.2.0.s390x.xml | 1 + .../caps_4.2.0.x86_64.xml | 1 + .../caps_5.0.0.aarch64.xml| 1 + .../qemucapabilitiesdata/caps_5.0.0.ppc64.xml | 1 + .../caps_5.0.0.riscv64.xml| 1 + .../caps_5.0.0.x86_64.xml | 1 + .../qemucapabilitiesdata/caps_5.1.0.sparc.xml | 1 + .../caps_5.1.0.x86_64.xml | 1 + .../caps_5.2.0.aarch64.xml| 1 + .../qemucapabilitiesdata/caps_5.2.0.ppc64.xml | 1 + .../caps_5.2.0.riscv64.xml| 1 + .../qemucapabilitiesdata/caps_5.2.0.s390x.xml | 1 + .../caps_5.2.0.x86_64.xml | 1 + .../caps_6.0.0.x86_64.xml | 1 + tests/qemucommandutiltest.c | 10 +- .../aarch64-gic-default-both.args | 28 ++- .../aarch64-gic-default-v2.args | 28 ++- .../aarch64-gic-default-v3.args | 28 ++- .../qemuxml2argvdata/aarch64-gic-default.args | 28 ++- .../aarch64-gic-none-both.args| 28 ++- .../qemuxml2argvdata/aarch64-gic-none-v2.args | 28 ++- .../qemuxml2argvdata/aarch64-gic-none-v3.args | 28 ++- tests/qemuxml2argvdata/aarch64-gic-none.args | 28 ++- tests/qemuxml2argvdata/cpu-check-full.args| 30 +++- tests/qemuxml2argvdata/cpu-check-partial.args | 30 +++- .../qemuxml2argvdata/cpu-numa-memshared.args | 4 +- ...sk-backing-chains-index.x86_64-2.12.0.args | 62 ++- ...sk-backing-chains-index.x86_64-latest.args | 169 +- ...isk-network-tlsx509-nbd.x86_64-2.12.0.args | 2 +- ...isk-network-tlsx509-nbd.x86_64-latest.args | 2 +- ...sk-network-tlsx509-vxhs.x86_64-2.12.0.args | 4 +- ...isk-network-tlsx509-vxhs.x86_64-5.0.0.args | 4 +- .../disk-network-tlsx509.x86_64-2.12.0.args | 6 +- .../disk-network-tlsx509.x86_64-latest.args | 6 +- .../disk-vhostuser.x86_64-latest.args | 4 +- .../fd-memory-numa-topology.args | 2 +- .../fd-memory-numa-topology2.args | 4 +- .../fd-memory-numa-topology3.args | 6 +- ...graphics-vnc-tls-secret.x86_64-latest.args | 4 +- ..
[libvirt PATCH 3/5] qemu: use on|off instead of yes|no for -object boolean properties
QEMU has long accepted many different values for boolean properties, but set accepted has been different depending on which QEMU parser you hit. The on|off values were supported by all QEMU parsers. The yes|no, y|n, true|false values were only partially supported: https://lists.gnu.org/archive/html/qemu-devel/2020-11/msg01012.html Thus we should standardize on on|off everywhere since that is most widely supported in QEMU. Signed-off-by: Daniel P. Berrangé --- src/util/virqemu.c| 2 +- .../aarch64-gic-default-both.args | 28 ++- .../aarch64-gic-default-v2.args | 28 ++- .../aarch64-gic-default-v3.args | 28 ++- .../qemuxml2argvdata/aarch64-gic-default.args | 28 ++- .../aarch64-gic-none-both.args| 28 ++- .../qemuxml2argvdata/aarch64-gic-none-v2.args | 28 ++- .../qemuxml2argvdata/aarch64-gic-none-v3.args | 28 ++- tests/qemuxml2argvdata/aarch64-gic-none.args | 28 ++- tests/qemuxml2argvdata/cpu-check-full.args| 30 +++- tests/qemuxml2argvdata/cpu-check-partial.args | 30 +++- .../qemuxml2argvdata/cpu-numa-memshared.args | 4 +- ...sk-backing-chains-index.x86_64-2.12.0.args | 62 ++- ...sk-backing-chains-index.x86_64-latest.args | 169 +- ...isk-network-tlsx509-nbd.x86_64-2.12.0.args | 2 +- ...isk-network-tlsx509-nbd.x86_64-latest.args | 2 +- ...sk-network-tlsx509-vxhs.x86_64-2.12.0.args | 4 +- ...isk-network-tlsx509-vxhs.x86_64-5.0.0.args | 4 +- .../disk-network-tlsx509.x86_64-2.12.0.args | 6 +- .../disk-network-tlsx509.x86_64-latest.args | 6 +- .../disk-vhostuser.x86_64-latest.args | 4 +- .../fd-memory-numa-topology.args | 2 +- .../fd-memory-numa-topology2.args | 4 +- .../fd-memory-numa-topology3.args | 6 +- ...graphics-vnc-tls-secret.x86_64-latest.args | 2 +- .../graphics-vnc-tls.x86_64-latest.args | 2 +- .../qemuxml2argvdata/hugepages-memaccess.args | 10 +- .../hugepages-memaccess2.args | 10 +- .../hugepages-memaccess3.x86_64-latest.args | 4 +- .../hugepages-numa-nodeset-part.args | 2 +- .../hugepages-numa-nodeset.args | 8 +- .../hugepages-nvdimm.x86_64-latest.args | 6 +- tests/qemuxml2argvdata/hugepages-shared.args | 8 +- .../mach-virt-console-native.args | 29 ++- .../mach-virt-serial+console-native.args | 29 ++- .../mach-virt-serial-compat.args | 29 ++- ...memory-default-hugepage.x86_64-latest.args | 4 +- .../memfd-memory-numa.x86_64-latest.args | 4 +- .../memory-hotplug-dimm-addr.args | 2 +- .../qemuxml2argvdata/memory-hotplug-dimm.args | 2 +- ...y-hotplug-nvdimm-access.x86_64-latest.args | 4 +- ...ry-hotplug-nvdimm-align.x86_64-latest.args | 4 +- ...ry-hotplug-nvdimm-label.x86_64-latest.args | 4 +- ...ory-hotplug-nvdimm-pmem.x86_64-latest.args | 4 +- ...emory-hotplug-nvdimm-ppc64-abi-update.args | 2 +- .../memory-hotplug-nvdimm-ppc64.args | 2 +- ...hotplug-nvdimm-readonly.x86_64-latest.args | 4 +- .../memory-hotplug-nvdimm.x86_64-latest.args | 2 +- ...ory-hotplug-virtio-pmem.x86_64-latest.args | 2 +- .../qemuxml2argvdata/pages-dimm-discard.args | 4 +- .../pci-rom-disabled-invalid.args | 30 +++- .../pseries-console-native.args | 28 ++- .../pseries-serial+console-native.args| 28 ++- .../pseries-serial-compat.args| 28 ++- .../serial-tcp-tlsx509-chardev-verify.args| 2 +- .../serial-tcp-tlsx509-chardev.args | 2 +- .../serial-tcp-tlsx509-secret-chardev.args| 2 +- .../shmem-plain-doorbell.args | 6 +- tests/qemuxml2argvdata/user-aliases.args | 8 +- tests/qemuxml2argvdata/user-aliases2.args | 29 ++- ...vhost-user-fs-fd-memory.x86_64-latest.args | 2 +- ...vhost-user-fs-hugepages.x86_64-latest.args | 2 +- ...host-user-gpu-secondary.x86_64-latest.args | 2 +- .../vhost-user-vga.x86_64-latest.args | 2 +- 64 files changed, 810 insertions(+), 105 deletions(-) mode change 12 => 100644 tests/qemuxml2argvdata/aarch64-gic-default-both.args mode change 12 => 100644 tests/qemuxml2argvdata/aarch64-gic-default-v2.args mode change 12 => 100644 tests/qemuxml2argvdata/aarch64-gic-default-v3.args mode change 12 => 100644 tests/qemuxml2argvdata/aarch64-gic-default.args mode change 12 => 100644 tests/qemuxml2argvdata/aarch64-gic-none-both.args mode change 12 => 100644 tests/qemuxml2argvdata/aarch64-gic-none-v2.args mode change 12 => 100644 tests/qemuxml2argvdata/aarch64-gic-none-v3.args mode change 12 => 100644 tests/qemuxml2argvdata/aarch64-gic-none.args mode change 12 => 100644 tests/qemuxml2argvdata/cpu-check-full.args mode change 12 => 100644 tests/qemuxml2argvdata/cpu-check-partial.args mode change 12 => 100644 tests/qemuxml2argvdata/disk-back
[libvirt PATCH 1/5] qemu: probe for -vnc supporting use of QemuOpts syntax
This was introduced in QEMU 2.2.0, and is visible by -vnc appearing in
the "query-command-line-options" data.
Signed-off-by: Daniel P. Berrangé
---
src/qemu/qemu_capabilities.c | 2 ++
src/qemu/qemu_capabilities.h | 1 +
tests/qemucapabilitiesdata/caps_2.10.0.aarch64.xml | 1 +
tests/qemucapabilitiesdata/caps_2.10.0.ppc64.xml | 1 +
tests/qemucapabilitiesdata/caps_2.10.0.s390x.xml | 1 +
tests/qemucapabilitiesdata/caps_2.10.0.x86_64.xml | 1 +
tests/qemucapabilitiesdata/caps_2.11.0.s390x.xml | 1 +
tests/qemucapabilitiesdata/caps_2.11.0.x86_64.xml | 1 +
tests/qemucapabilitiesdata/caps_2.12.0.aarch64.xml | 1 +
tests/qemucapabilitiesdata/caps_2.12.0.ppc64.xml | 1 +
tests/qemucapabilitiesdata/caps_2.12.0.s390x.xml | 1 +
tests/qemucapabilitiesdata/caps_2.12.0.x86_64.xml | 1 +
tests/qemucapabilitiesdata/caps_2.4.0.x86_64.xml | 1 +
tests/qemucapabilitiesdata/caps_2.5.0.x86_64.xml | 1 +
tests/qemucapabilitiesdata/caps_2.6.0.aarch64.xml | 1 +
tests/qemucapabilitiesdata/caps_2.6.0.ppc64.xml| 1 +
tests/qemucapabilitiesdata/caps_2.6.0.x86_64.xml | 1 +
tests/qemucapabilitiesdata/caps_2.7.0.s390x.xml| 1 +
tests/qemucapabilitiesdata/caps_2.7.0.x86_64.xml | 1 +
tests/qemucapabilitiesdata/caps_2.8.0.s390x.xml| 1 +
tests/qemucapabilitiesdata/caps_2.8.0.x86_64.xml | 1 +
tests/qemucapabilitiesdata/caps_2.9.0.ppc64.xml| 1 +
tests/qemucapabilitiesdata/caps_2.9.0.s390x.xml| 1 +
tests/qemucapabilitiesdata/caps_2.9.0.x86_64.xml | 1 +
tests/qemucapabilitiesdata/caps_3.0.0.ppc64.xml| 1 +
tests/qemucapabilitiesdata/caps_3.0.0.riscv32.xml | 1 +
tests/qemucapabilitiesdata/caps_3.0.0.riscv64.xml | 1 +
tests/qemucapabilitiesdata/caps_3.0.0.s390x.xml| 1 +
tests/qemucapabilitiesdata/caps_3.0.0.x86_64.xml | 1 +
tests/qemucapabilitiesdata/caps_3.1.0.ppc64.xml| 1 +
tests/qemucapabilitiesdata/caps_3.1.0.x86_64.xml | 1 +
tests/qemucapabilitiesdata/caps_4.0.0.aarch64.xml | 1 +
tests/qemucapabilitiesdata/caps_4.0.0.ppc64.xml| 1 +
tests/qemucapabilitiesdata/caps_4.0.0.riscv32.xml | 1 +
tests/qemucapabilitiesdata/caps_4.0.0.riscv64.xml | 1 +
tests/qemucapabilitiesdata/caps_4.0.0.s390x.xml| 1 +
tests/qemucapabilitiesdata/caps_4.0.0.x86_64.xml | 1 +
tests/qemucapabilitiesdata/caps_4.1.0.x86_64.xml | 1 +
tests/qemucapabilitiesdata/caps_4.2.0.aarch64.xml | 1 +
tests/qemucapabilitiesdata/caps_4.2.0.ppc64.xml| 1 +
tests/qemucapabilitiesdata/caps_4.2.0.s390x.xml| 1 +
tests/qemucapabilitiesdata/caps_4.2.0.x86_64.xml | 1 +
tests/qemucapabilitiesdata/caps_5.0.0.aarch64.xml | 1 +
tests/qemucapabilitiesdata/caps_5.0.0.ppc64.xml| 1 +
tests/qemucapabilitiesdata/caps_5.0.0.riscv64.xml | 1 +
tests/qemucapabilitiesdata/caps_5.0.0.x86_64.xml | 1 +
tests/qemucapabilitiesdata/caps_5.1.0.sparc.xml| 1 +
tests/qemucapabilitiesdata/caps_5.1.0.x86_64.xml | 1 +
tests/qemucapabilitiesdata/caps_5.2.0.aarch64.xml | 1 +
tests/qemucapabilitiesdata/caps_5.2.0.ppc64.xml| 1 +
tests/qemucapabilitiesdata/caps_5.2.0.riscv64.xml | 1 +
tests/qemucapabilitiesdata/caps_5.2.0.s390x.xml| 1 +
tests/qemucapabilitiesdata/caps_5.2.0.x86_64.xml | 1 +
tests/qemucapabilitiesdata/caps_6.0.0.x86_64.xml | 1 +
54 files changed, 55 insertions(+)
diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c
index 46b0109ca2..3f8593a9e5 100644
--- a/src/qemu/qemu_capabilities.c
+++ b/src/qemu/qemu_capabilities.c
@@ -616,6 +616,7 @@ VIR_ENUM_IMPL(virQEMUCaps,
"vhost-user-blk",
"cpu-max",
"memory-backend-file.x-use-canonical-path-for-ramblock-id",
+ "vnc-opts",
);
@@ -3295,6 +3296,7 @@ static struct virQEMUCapsCommandLineProps
virQEMUCapsCommandLine[] = {
{ "fsdev", "multidevs", QEMU_CAPS_FSDEV_MULTIDEVS },
{ "fw_cfg", "file", QEMU_CAPS_FW_CFG },
{ "fsdev", "fmode", QEMU_CAPS_FSDEV_CREATEMODE }, /* Could have also
checked fsdev->dmode */
+{ "vnc", "display", QEMU_CAPS_VNC_OPTS },
};
static int
diff --git a/src/qemu/qemu_capabilities.h b/src/qemu/qemu_capabilities.h
index 6fd7922926..38574eef16 100644
--- a/src/qemu/qemu_capabilities.h
+++ b/src/qemu/qemu_capabilities.h
@@ -596,6 +596,7 @@ typedef enum { /* virQEMUCapsFlags grouping marker for
syntax-check */
QEMU_CAPS_DEVICE_VHOST_USER_BLK, /* -device vhost-user-blk */
QEMU_CAPS_CPU_MAX, /* -cpu max */
QEMU_CAPS_X_USE_CANONICAL_PATH_FOR_RAMBLOCK_ID, /* -object
memory-backend-file,x-use-canonical-path-for-ramblock-id= */
+QEMU_CAPS_VNC_OPTS, /* -vnc uses QemuOpts parser instead of custom code */
QEMU_CAPS_LAST /* this must always be the last item */
} virQEMUCapsFlags;
diff --git a/tests/qemucapabilitiesdata/caps_2.10.0.aarch64.xml
b/tests/qemucapabilitiesdata/caps_2.10.0.aarch64.xml
index 5b7df57b50..cce5233a14 100644
--- a/tests/qemucapabilitiesdata/caps_2.10.0.aarch64.xml
+++ b/tests/qemucapabilitiesdata/caps_2.10
Re: [libvirt PATCH v4 09/25] nodedev: add mdevctl devices to node device list
On Wed, Feb 03, 2021 at 11:38:53AM -0600, Jonathon Jongsma wrote:
> At startup, query devices that are defined by 'mdevctl' and add them to
> the node device list.
>
> This adds a complication: we now have two potential sources of
> information for a node device:
> - udev for all devices and for activated mediated devices
> - mdevctl for persistent mediated devices
>
> Unfortunately, neither backend returns full information for a mediated
> device. For example, if a persistent mediated device in the list (with
> information provided from mdevctl) is 'started', that same device will
> now be detected by udev. If we simply overwrite the existing device
> definition with the new one provided by the udev backend, we will lose
> extra information that was provided by mdevctl (e.g. attributes, etc).
> To avoid this, make sure to copy the extra information into the new
> device definition.
>
> Signed-off-by: Jonathon Jongsma
> ---
> src/node_device/node_device_driver.c | 164 +++
> src/node_device/node_device_driver.h | 6 +
> src/node_device/node_device_udev.c | 31 -
> 3 files changed, 196 insertions(+), 5 deletions(-)
>
> diff --git a/src/node_device/node_device_driver.c
> b/src/node_device/node_device_driver.c
> index 2778e41f97..fd57dcacc1 100644
> --- a/src/node_device/node_device_driver.c
> +++ b/src/node_device/node_device_driver.c
> @@ -1156,3 +1156,167 @@ nodeDeviceGenerateName(virNodeDeviceDefPtr def,
> *(def->name + i) = '_';
> }
> }
> +
> +
> +static int
> +virMdevctlListDefined(virNodeDeviceDefPtr **devs)
> +{
> +int status;
> +g_autofree char *output = NULL;
> +g_autoptr(virCommand) cmd = nodeDeviceGetMdevctlListCommand(true,
> &output);
> +
> +if (virCommandRun(cmd, &status) < 0)
> +return -1;
> +
> +if (!output)
> +return -1;
> +
> +return nodeDeviceParseMdevctlJSON(output, devs);
> +}
> +
> +
> +typedef struct _virMdevctlForEachData virMdevctlForEachData;
> +struct _virMdevctlForEachData {
> +int ndefs;
> +virNodeDeviceDefPtr *defs;
> +};
> +
^This struct doesn't seem to be used anywhere in the patch, so please define
it in the patch that makes use of it.
> +
> +int
> +nodeDeviceUpdateMediatedDevices(void)
> +{
> +g_autofree virNodeDeviceDefPtr *defs = NULL;
> +int ndefs;
> +GList * tofree = NULL;
> +size_t i;
> +
> +if ((ndefs = virMdevctlListDefined(&defs)) < 0) {
> +virReportSystemError(errno, "%s",
> + _("failed to query mdevs from mdevctl"));
> +return -1;
> +}
> +
> +for (i = 0; i < ndefs; i++) {
> +virNodeDeviceObjPtr obj;
> +virObjectEventPtr event;
> +virNodeDeviceDefPtr def = defs[i];
> +bool was_defined = false;
> +
> +def->driver = g_strdup("vfio_mdev");
> +
> +if ((obj = virNodeDeviceObjListFindByName(driver->devs, def->name)))
> {
^This condition should be flipped so that the 'else' block becomes the
larger one.
> +bool changed;
> +virNodeDeviceDefPtr olddef = virNodeDeviceObjGetDef(obj);
> +
> +was_defined = virNodeDeviceObjIsPersistent(obj);
"defined" as a name will do just fine
> +/* Active devices contain some additional information (e.g. sysfs
> + * path) that is not provided by mdevctl, so re-use the existing
> + * definition and copy over new mdev data */
> +changed = nodeDeviceDefCopyFromMdevctl(olddef, def);
> +
> +/* Re-use the existing definition (after merging new data from
> + * mdevctl), so def needs to be freed at the end of the function
> */
> +tofree = g_list_prepend(tofree, def);
I'm not a fan of ^this. Plain g_autoptr on virNodeDeviceDef would do just
fine.
> +
> +if (was_defined && !changed) {
> +/* if this device was already defined and the definition
> + * hasn't changed, there's nothing to do for this device */
> +virNodeDeviceObjEndAPI(&obj);
> +continue;
> +}
> +} else {
> +if (!(obj = virNodeDeviceObjListAssignDef(driver->devs, def))) {
> +virNodeDeviceDefFree(def);
> +goto cleanup;
> +}
> +}
> +
> +/* all devices returned by virMdevctlListDefined() are persistent */
> +virNodeDeviceObjSetPersistent(obj, true);
> +
> +if (!was_defined)
> +event = virNodeDeviceEventLifecycleNew(def->name,
> +
> VIR_NODE_DEVICE_EVENT_DEFINED,
> + 0);
> +else
> +event = virNodeDeviceEventUpdateNew(def->name);
> +
> +virNodeDeviceObjEndAPI(&obj);
> +virObjectEventStateQueue(driver->nodeDeviceEventState, event);
> +}
> +
> + cleanup:
> +g_list_free_full(tofree, (GDestroy
Re: [PATCH] qemu: use long on|off syntax for -spice boolean option values
On a Tuesday in 2021, Daniel P. Berrangé wrote: The preferred syntax for boolean options is to set the value "on" or "off". QEMU 7.1.0 will deprecate the short format we currently use. The long format has been supported with -spice since at least 1.5.3, so we don't need to check for it. Signed-off-by: Daniel P. Berrangé --- src/qemu/qemu_command.c | 8 .../qemuxml2argvdata/graphics-spice-agent-file-xfer.args | 2 +- tests/qemuxml2argvdata/graphics-spice-sasl.args | 2 +- tests/qemuxml2argvdata/graphics-spice-usb-redir.args | 2 +- tests/qemuxml2argvdata/graphics-spice.args| 2 +- 5 files changed, 8 insertions(+), 8 deletions(-) Reviewed-by: Ján Tomko Jano signature.asc Description: PGP signature
Re: [libvirt PATCH] qemu: use long on|off syntax for -chardev boolean option values
On a Tuesday in 2021, Daniel P. Berrangé wrote: The preferred syntax for boolean options is to set the value "on" or "off". QEMU 7.1.0 will deprecate the short format we currently use. The long format has been supported with -chardev since at least 1.5.3, so we don't need to check for it. Signed-off-by: Daniel P. Berrangé --- src/qemu/qemu_command.c | 10 +- tests/qemuxml2argvdata/aarch64-aavmf-virtio-mmio.args | 2 +- tests/qemuxml2argvdata/aarch64-acpi-uefi.args | 2 +- [...] .../x86_64-q35-graphics.x86_64-latest.args| 4 ++-- .../x86_64-q35-headless.x86_64-latest.args| 4 ++-- 835 files changed, 896 insertions(+), 893 deletions(-) Reviewed-by: Ján Tomko Jano signature.asc Description: PGP signature
Re: [libvirt PATCH v4 07/25] nodedev: add persistence to virNodeDeviceObj
On Wed, Feb 03, 2021 at 11:38:51AM -0600, Jonathon Jongsma wrote:
> Consistent with other objects (e.g. virDomainObj), add a field to
> indicate whether the node device is persistent or transient.
>
> Signed-off-by: Jonathon Jongsma
> ---
> src/conf/virnodedeviceobj.c | 14 ++
> src/conf/virnodedeviceobj.h | 6 ++
> src/libvirt_private.syms| 2 ++
> 3 files changed, 22 insertions(+)
>
> diff --git a/src/conf/virnodedeviceobj.c b/src/conf/virnodedeviceobj.c
> index 6e9291264a..53cf3d1bcd 100644
> --- a/src/conf/virnodedeviceobj.c
> +++ b/src/conf/virnodedeviceobj.c
> @@ -40,6 +40,7 @@ struct _virNodeDeviceObj {
> bool skipUpdateCaps;/* whether to skip checking host
> caps,
> used by testdriver */
> bool active;
> +bool persistent;
> };
>
> struct _virNodeDeviceObjList {
> @@ -1000,3 +1001,16 @@ virNodeDeviceObjSetActive(virNodeDeviceObjPtr obj,
> {
> obj->active = active;
> }
2 blank lines in between function definitions please.
Reviewed-by: Erik Skultety
> +
> +bool
> +virNodeDeviceObjIsPersistent(virNodeDeviceObjPtr obj)
> +{
> +return obj->persistent;
> +}
> +
> +void
> +virNodeDeviceObjSetPersistent(virNodeDeviceObjPtr obj,
> + bool persistent)
> +{
> +obj->persistent = persistent;
> +}
> diff --git a/src/conf/virnodedeviceobj.h b/src/conf/virnodedeviceobj.h
> index c119f4c51f..43af012103 100644
> --- a/src/conf/virnodedeviceobj.h
> +++ b/src/conf/virnodedeviceobj.h
> @@ -128,3 +128,9 @@ virNodeDeviceObjIsActive(virNodeDeviceObjPtr obj);
> void
> virNodeDeviceObjSetActive(virNodeDeviceObjPtr obj,
>bool active);
> +bool
> +virNodeDeviceObjIsPersistent(virNodeDeviceObjPtr obj);
> +
> +void
> +virNodeDeviceObjSetPersistent(virNodeDeviceObjPtr obj,
> + bool persistent);
> diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
> index c8ef6fe983..d044ab 100644
> --- a/src/libvirt_private.syms
> +++ b/src/libvirt_private.syms
> @@ -1262,6 +1262,7 @@ virNetworkPortDefSaveStatus;
> virNodeDeviceObjEndAPI;
> virNodeDeviceObjGetDef;
> virNodeDeviceObjIsActive;
> +virNodeDeviceObjIsPersistent;
> virNodeDeviceObjListAssignDef;
> virNodeDeviceObjListExport;
> virNodeDeviceObjListFindByName;
> @@ -1275,6 +1276,7 @@ virNodeDeviceObjListNew;
> virNodeDeviceObjListNumOfDevices;
> virNodeDeviceObjListRemove;
> virNodeDeviceObjSetActive;
> +virNodeDeviceObjSetPersistent;
>
>
> # conf/virnwfilterbindingdef.h
> --
> 2.26.2
>
Re: [libvirt PATCH v4 02/25] nodedev: introduce concept of 'active' node devices
...
> struct _virNodeDeviceObjList {
> @@ -976,3 +977,16 @@ virNodeDeviceObjSetSkipUpdateCaps(virNodeDeviceObjPtr
> obj,
> {
> obj->skipUpdateCaps = skipUpdateCaps;
> }
2 blank lines in between the function definitions please.
> +
> +bool
> +virNodeDeviceObjIsActive(virNodeDeviceObjPtr obj)
> +{
> +return obj->active;
> +}
> +
> +void
> +virNodeDeviceObjSetActive(virNodeDeviceObjPtr obj,
> + bool active)
> +{
> +obj->active = active;
> +}
Erik
[PATCH] qemu: use long on|off syntax for -spice boolean option values
The preferred syntax for boolean options is to set the value "on" or
"off". QEMU 7.1.0 will deprecate the short format we currently use.
The long format has been supported with -spice since at least 1.5.3,
so we don't need to check for it.
Signed-off-by: Daniel P. Berrangé
---
src/qemu/qemu_command.c | 8
.../qemuxml2argvdata/graphics-spice-agent-file-xfer.args | 2 +-
tests/qemuxml2argvdata/graphics-spice-sasl.args | 2 +-
tests/qemuxml2argvdata/graphics-spice-usb-redir.args | 2 +-
tests/qemuxml2argvdata/graphics-spice.args| 2 +-
5 files changed, 8 insertions(+), 8 deletions(-)
diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
index 1d2fc4495b..62fae6fe81 100644
--- a/src/qemu/qemu_command.c
+++ b/src/qemu/qemu_command.c
@@ -7779,7 +7779,7 @@ qemuBuildGraphicsSPICECommandLine(virQEMUDriverConfigPtr
cfg,
}
if (cfg->spiceSASL) {
-virBufferAddLit(&opt, "sasl,");
+virBufferAddLit(&opt, "sasl=on,");
if (cfg->spiceSASLdir)
virCommandAddEnvPair(cmd, "SASL_CONF_PATH",
@@ -7811,7 +7811,7 @@ qemuBuildGraphicsSPICECommandLine(virQEMUDriverConfigPtr
cfg,
* in this bit of the code */
if (!graphics->data.spice.auth.passwd &&
!cfg->spicePassword)
-virBufferAddLit(&opt, "disable-ticketing,");
+virBufferAddLit(&opt, "disable-ticketing=on,");
if (hasSecure) {
virBufferAddLit(&opt, "x509-dir=");
@@ -7893,10 +7893,10 @@
qemuBuildGraphicsSPICECommandLine(virQEMUDriverConfigPtr cfg,
virBufferAsprintf(&opt, "streaming-video=%s,",
virDomainGraphicsSpiceStreamingModeTypeToString(graphics->data.spice.streaming));
if (graphics->data.spice.copypaste == VIR_TRISTATE_BOOL_NO)
-virBufferAddLit(&opt, "disable-copy-paste,");
+virBufferAddLit(&opt, "disable-copy-paste=on,");
if (graphics->data.spice.filetransfer == VIR_TRISTATE_BOOL_NO)
-virBufferAddLit(&opt, "disable-agent-file-xfer,");
+virBufferAddLit(&opt, "disable-agent-file-xfer=on,");
if (graphics->data.spice.gl == VIR_TRISTATE_BOOL_YES) {
/* spice.gl is a TristateBool, but qemu expects on/off: use
diff --git a/tests/qemuxml2argvdata/graphics-spice-agent-file-xfer.args
b/tests/qemuxml2argvdata/graphics-spice-agent-file-xfer.args
index 5aa7da2d4a..08c3173e6a 100644
--- a/tests/qemuxml2argvdata/graphics-spice-agent-file-xfer.args
+++ b/tests/qemuxml2argvdata/graphics-spice-agent-file-xfer.args
@@ -27,7 +27,7 @@ server=on,wait=off \
-drive file=/dev/HostVG/QEMUGuest1,format=raw,if=none,id=drive-ide0-0-0 \
-device ide-hd,bus=ide.0,unit=0,drive=drive-ide0-0-0,id=ide0-0-0,bootindex=1 \
-spice port=5903,tls-port=5904,addr=127.0.0.1,x509-dir=/etc/pki/libvirt-spice,\
-tls-channel=main,plaintext-channel=inputs,disable-agent-file-xfer,\
+tls-channel=main,plaintext-channel=inputs,disable-agent-file-xfer=on,\
seamless-migration=on \
-vga qxl \
-global qxl-vga.ram_size=67108864 \
diff --git a/tests/qemuxml2argvdata/graphics-spice-sasl.args
b/tests/qemuxml2argvdata/graphics-spice-sasl.args
index d71c527a09..024b775afd 100644
--- a/tests/qemuxml2argvdata/graphics-spice-sasl.args
+++ b/tests/qemuxml2argvdata/graphics-spice-sasl.args
@@ -27,7 +27,7 @@ server=on,wait=off \
-usb \
-drive file=/dev/HostVG/QEMUGuest1,format=raw,if=none,id=drive-ide0-0-0 \
-device ide-hd,bus=ide.0,unit=0,drive=drive-ide0-0-0,id=ide0-0-0,bootindex=1 \
--spice port=5903,tls-port=5904,addr=127.0.0.1,sasl,\
+-spice port=5903,tls-port=5904,addr=127.0.0.1,sasl=on,\
x509-dir=/etc/pki/libvirt-spice,tls-channel=default,seamless-migration=on \
-vga qxl \
-global qxl-vga.ram_size=67108864 \
diff --git a/tests/qemuxml2argvdata/graphics-spice-usb-redir.args
b/tests/qemuxml2argvdata/graphics-spice-usb-redir.args
index ba6e6abee7..780e189a95 100644
--- a/tests/qemuxml2argvdata/graphics-spice-usb-redir.args
+++ b/tests/qemuxml2argvdata/graphics-spice-usb-redir.args
@@ -32,7 +32,7 @@ addr=0x4 \
tls-channel=main,plaintext-channel=inputs,tls-channel=usbredir,\
image-compression=auto_glz,jpeg-wan-compression=auto,\
zlib-glz-wan-compression=auto,playback-compression=on,streaming-video=filter,\
-disable-copy-paste,seamless-migration=on \
+disable-copy-paste=on,seamless-migration=on \
-vga cirrus \
-chardev socket,id=charredir0,host=localhost,port=4000 \
-device usb-redir,chardev=charredir0,id=redir0,bus=usb.0,port=1 \
diff --git a/tests/qemuxml2argvdata/graphics-spice.args
b/tests/qemuxml2argvdata/graphics-spice.args
index 34f3a7dd33..e2a8593c8e 100644
--- a/tests/qemuxml2argvdata/graphics-spice.args
+++ b/tests/qemuxml2argvdata/graphics-spice.args
@@ -30,7 +30,7 @@ server=on,wait=off \
tls-channel=default,tls-channel=main,plaintext-channel=inputs,\
image-compression=auto_glz,jpeg-wan-compression=auto,\
zlib-glz-wan-compression=auto,playback-compression=on,streaming-video=filter,\
-disable-copy-paste,disable-agent-file-xfer,
Re: [libvirt PATCH 0/2] ci: Build on macOS 11
On a Monday in 2021, Andrea Bolognani wrote:
Test pipeline: https://gitlab.com/abologna/libvirt/-/pipelines/256476242
Andrea Bolognani (2):
ci: Update package list on Cirrus CI
ci: Build on macOS 11 instead of macOS 10.15
.gitlab-ci.yml | 10 +++---
ci/cirrus/build.yml | 1 +
ci/cirrus/{macos-1015.vars => macos-11.vars} | 0
3 files changed, 8 insertions(+), 3 deletions(-)
rename ci/cirrus/{macos-1015.vars => macos-11.vars} (100%)
Reviewed-by: Ján Tomko
Jano
signature.asc
Description: PGP signature
Re: [PATCH 2/3] qemu: Move qemuAgentFSInfo array free into qemuDomainGetFSInfo()
On a Monday in 2021, Michal Privoznik wrote:
On 2/15/21 6:00 PM, Ján Tomko wrote:
On a Monday in 2021, Michal Privoznik wrote:
When qemuDomainGetFSInfo() is called it calls
qemuDomainGetFSInfoAgent() which executes 'guest-get-fsinfo'
guest agent command, parses returned JSON and returns an array of
qemuAgentFSInfo structures (well, pointers to those structs).
Then it grabs a domain job and tries to do some matching of guest
returned info against domain definition. This matching is done in
virDomainFSInfoFormat() which also frees the array of
qemuAgentFSInfo structures allocated earlier.
But this is not just. If acquiring the domain job fails (or
domain activeness check executed right after that fails) then
virDomainFSInfoFormat() is not called, leaking the array of
structs.
Signed-off-by: Michal Privoznik
---
src/qemu/qemu_driver.c | 21 +
1 file changed, 13 insertions(+), 8 deletions(-)
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index f59f9e13ba..d30cf75b73 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -18977,14 +18977,14 @@ virDomainFSInfoFormat(qemuAgentFSInfoPtr
*agentinfo,
ret = nagentinfo;
cleanup:
- for (i = 0; i < nagentinfo; i++) {
- qemuAgentFSInfoFree(agentinfo[i]);
- /* if there was an error, free any memory we've allocated
for the
- * return value */
- if (info_ret)
+ if (info_ret) {
+ for (i = 0; i < nagentinfo; i++) {
+ /* if there was an error, free any memory we've
allocated for the
+ * return value */
virDomainFSInfoFree(info_ret[i]);
+ }
+ g_free(info_ret);
}
- g_free(info_ret);
return ret;
}
This hunk is unrelated and just cosmetic.
I'm not sure what you mean by 'unrelated'. The freeing must be moved
in one commit, otherwise either the array is leaked or double freed.
Ah, now I see that little that little line hidden in there:
-qemuAgentFSInfoFree(agentinfo[i]);
I thought all it did was exchange the 'if' and 'for'.
Please leave that line in this commit and separate the exchange.
Jano
Michal
signature.asc
Description: PGP signature
Re: [PATCH 3/3] qemuSnapshotFSFreeze: Don't return -2
On Tue, Feb 16, 2021 at 09:46:42AM +0100, Peter Krempa wrote: > On Mon, Feb 15, 2021 at 19:20:25 +0100, Pavel Hrdina wrote: > > On Mon, Feb 15, 2021 at 06:27:51PM +0100, Peter Krempa wrote: > > > The -2 value is misleading because if 'qemuAgentFSFreeze' fails it > > > doesn't necessarily mean that the command was sent to the agent. > > > > > > Since callers don't care about the -2 value specifically, remove it. > > > > In addition this indirectly fixes virDomainFSFreeze public API where > > we return result of qemuSnapshotFSFreeze directly. Now we comply with > > the API description. > > Luckily -2 happens only internally, so we never broke any public API > promise: > > This is caused by virNetServerProgramDispatchCall treating any negative > value as error and transporting it over RPC and then > virNetClientProgramCall in the client returning -1 if the returned reply > is of VIR_NET_ERROR type. Well, lucky for us :) I did not count RPC and the fact that even local connection will use remote driver for QEMU. Pavel signature.asc Description: PGP signature
Re: [PATCH 3/3] qemuSnapshotFSFreeze: Don't return -2
On Mon, Feb 15, 2021 at 19:20:25 +0100, Pavel Hrdina wrote:
> On Mon, Feb 15, 2021 at 06:27:51PM +0100, Peter Krempa wrote:
> > The -2 value is misleading because if 'qemuAgentFSFreeze' fails it
> > doesn't necessarily mean that the command was sent to the agent.
> >
> > Since callers don't care about the -2 value specifically, remove it.
>
> In addition this indirectly fixes virDomainFSFreeze public API where
> we return result of qemuSnapshotFSFreeze directly. Now we comply with
> the API description.
Luckily -2 happens only internally, so we never broke any public API
promise:
This is caused by virNetServerProgramDispatchCall treating any negative
value as error and transporting it over RPC and then
virNetClientProgramCall in the client returning -1 if the returned reply
is of VIR_NET_ERROR type.
Thread 1 "virsh" hit Breakpoint 1, virDomainFSFreeze
(dom=dom@entry=0x5569b550, mountpoints=0x0, nmountpoints=0,
flags=flags@entry=0) at ../../../libvirt/src/libvirt-domain.c:11327
11327 {
(gdb) next
11328 VIR_DOMAIN_DEBUG(dom, "mountpoints=%p, nmountpoints=%d, flags=0x%x",
(gdb)
11331 virResetLastError();
(gdb)
11333 virCheckDomainReturn(dom, -1);
(gdb)
11334 virCheckReadOnlyGoto(dom->conn->flags, error);
(gdb)
11335 virCheckNonNullArrayArgGoto(mountpoints, nmountpoints, error);
(gdb)
11337 if (dom->conn->driver->domainFSFreeze) {
(gdb)
11338 int ret = dom->conn->driver->domainFSFreeze(
(gdb)
11340 if (ret < 0)
(gdb) p ret
$1 = -1
(gdb) c
Continuing.
error: Unable to freeze filesystems
error: internal error: unable to execute QEMU agent command
'guest-fsfreeze-freeze': The command guest-fsfreeze-freeze has been disabled
for this instance
