[libvirt PATCH 0/3] gitdm: Fixes and updates
blurb.com *** BLURB HERE ***, Inc. Andrea Bolognani (3): gitdm: Add entry for example.com gitdm: Fix sorting gitdm: Add missing entries docs/gitdm/companies/others| 5 - docs/gitdm/groups/unaffiliated | 5 + 2 files changed, 9 insertions(+), 1 deletion(-) -- 2.24.1
[libvirt PATCH 1/3] gitdm: Add entry for example.com
We already have one instance of it being used in our git history, and more are probably bound to show up eventually. Signed-off-by: Andrea Bolognani --- docs/gitdm/groups/unaffiliated | 5 + 1 file changed, 5 insertions(+) diff --git a/docs/gitdm/groups/unaffiliated b/docs/gitdm/groups/unaffiliated index 70e82ba894..a58731a683 100644 --- a/docs/gitdm/groups/unaffiliated +++ b/docs/gitdm/groups/unaffiliated @@ -1,3 +1,8 @@ +# This domain will show up because of a mistake, and for that reason we +# can't really pin it to a specific company or community, so here it is :) + +example.com + # These are all domains you can get a personal email address from, so it's # fair to assume people using such addresses are contributing in their spare # time rather than on behalf of their respective employers. -- 2.24.1
[libvirt PATCH 3/3] gitdm: Add missing entries
A few new companies have contributed to libvirt since the last time the gitdm configuration was updated. Signed-off-by: Andrea Bolognani --- docs/gitdm/companies/others | 3 +++ 1 file changed, 3 insertions(+) diff --git a/docs/gitdm/companies/others b/docs/gitdm/companies/others index bf35273f4a..f459163dd2 100644 --- a/docs/gitdm/companies/others +++ b/docs/gitdm/companies/others @@ -10,6 +10,7 @@ av-test.de AV-TEST b1-systems.de B1 Systems baidu.com Baidu brightbox.co.uk Brightbox +bytedance.com ByteDance cisco.com Cisco citrix.com Citrix cloudwatt.com Cloudwatt @@ -38,11 +39,13 @@ hitachi.com Hitachi hoster-ok.com hoster-ok.com hp.com HP huawei.com Huawei +hupstream.com hupstream hygon.cn Hygon inktank.com Inktank Storage intel.com Intel intellilink.co.jp NTT DATA INTELLILINK invisiblethingslab.com Invisible Things Lab +ixsystems.com iXsystems jtan.com JTAN juniper.net Juniper Networks laposte.net La Poste -- 2.24.1
[libvirt PATCH 2/3] gitdm: Fix sorting
Fixes: 3a3a85c529e92ad767fbe01587186854c175 Signed-off-by: Andrea Bolognani --- docs/gitdm/companies/others | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/gitdm/companies/others b/docs/gitdm/companies/others index affbaf3ea2..bf35273f4a 100644 --- a/docs/gitdm/companies/others +++ b/docs/gitdm/companies/others @@ -1,5 +1,4 @@ 6wind.com 6WIND -cmss.chinamobile.com China Mobile active.by ActiveCloud aero.org Aerospace akamai.com Akamai @@ -14,6 +13,7 @@ brightbox.co.uk Brightbox cisco.com Cisco citrix.com Citrix cloudwatt.com Cloudwatt +cmss.chinamobile.com China Mobile codethink.co.uk Codethink cumulusnetworks.com Cumulus Networks dataductus.se Data Ductus -- 2.24.1
Re: [PATCH] admin: use g_autofree
On Fri, Feb 28, 2020 at 11:46:40PM +0530, Gaurav Agrawal wrote:
> From: GAURAV AGRAWAL
>
> Signed-off-by: Gaurav Agrawal
> ---
> src/admin/libvirt-admin.c | 15 +--
> 1 file changed, 5 insertions(+), 10 deletions(-)
>
> diff --git a/src/admin/libvirt-admin.c b/src/admin/libvirt-admin.c
> index 4099a54854..17d0eb39fe 100644
> --- a/src/admin/libvirt-admin.c
> +++ b/src/admin/libvirt-admin.c
> @@ -111,7 +111,7 @@ getSocketPath(virURIPtr uri)
> virURIParamPtr param = >params[i];
>
> if (STREQ(param->name, "socket")) {
> -VIR_FREE(sock_path);
> +g_free(sock_path);
> sock_path = g_strdup(param->value);
> } else {
> virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
> @@ -203,11 +203,11 @@ virAdmGetDefaultURI(virConfPtr conf, char **uristr)
> virAdmConnectPtr
> virAdmConnectOpen(const char *name, unsigned int flags)
> {
> -char *sock_path = NULL;
> +g_autofree char *sock_path = NULL;
> char *alias = NULL;
> virAdmConnectPtr conn = NULL;
> g_autoptr(virConf) conf = NULL;
> -char *uristr = NULL;
> +g_autofree char *uristr = NULL;
>
> if (virAdmInitialize() < 0)
> goto error;
> @@ -233,7 +233,7 @@ virAdmConnectOpen(const char *name, unsigned int flags)
> goto error;
>
> if (alias) {
> -VIR_FREE(uristr);
> +g_free(uristr);
> uristr = alias;
> }
>
> @@ -251,16 +251,11 @@ virAdmConnectOpen(const char *name, unsigned int flags)
> if (remoteAdminConnectOpen(conn, flags) < 0)
> goto error;
>
> - cleanup:
> -VIR_FREE(sock_path);
> -VIR_FREE(uristr);
> return conn;
>
> error:
> virDispatchError(NULL);
> -virObjectUnref(conn);
This line is still needed
> -conn = NULL;
> -goto cleanup;
> +return NULL;
egards,
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 3/8] qemu-conf: add dbusStateDir
On 2/25/20 6:24 AM, Michal Privoznik wrote:
On 2/24/20 4:57 PM, Marc-André Lureau wrote:
Hi
On Thu, Feb 20, 2020 at 10:04 AM Michal Privoznik
wrote:
On 1/14/20 2:46 PM, marcandre.lur...@redhat.com wrote:
From: Marc-André Lureau
Location of DBus daemon state configuration, socket, pid...
Signed-off-by: Marc-André Lureau
---
src/qemu/qemu_conf.c | 4
src/qemu/qemu_conf.h | 1 +
2 files changed, 5 insertions(+)
diff --git a/src/qemu/qemu_conf.c b/src/qemu/qemu_conf.c
index e1fea390c7..ade12dac6c 100644
--- a/src/qemu/qemu_conf.c
+++ b/src/qemu/qemu_conf.c
@@ -144,6 +144,8 @@ virQEMUDriverConfigPtr
virQEMUDriverConfigNew(bool privileged)
cfg->cacheDir = g_strdup_printf("%s/cache/libvirt/qemu",
LOCALSTATEDIR);
+ cfg->dbusStateDir =
g_strdup_printf("%s/run/libvirt/qemu/dbus", LOCALSTATEDIR);
+
cfg->libDir = g_strdup_printf("%s/lib/libvirt/qemu",
LOCALSTATEDIR);
cfg->saveDir = g_strdup_printf("%s/save", cfg->libDir);
cfg->snapshotDir = g_strdup_printf("%s/snapshot",
cfg->libDir);
@@ -174,6 +176,7 @@ virQEMUDriverConfigPtr
virQEMUDriverConfigNew(bool privileged)
cfg->stateDir = g_strdup_printf("%s/qemu/run", rundir);
cfg->swtpmStateDir = g_strdup_printf("%s/swtpm",
cfg->stateDir);
+ cfg->dbusStateDir = g_strdup_printf("%s/dbus",
cfg->stateDir);
cfg->configBaseDir = virGetUserConfigDirectory();
Instead of doing practically the same in two branches, you can achieve
the same result with just one line if you put the call just below
cfg->slirpStateDir init.
However, do we need this to be in a special directory instead of per VM
private directory? The way I implemented PR helper was that the socket
it creates and to which qemu connects is stored under vm->priv->libDir
which is initialized in qemuDomainSetPrivatePaths() to:
$cfg->libDir/domain-$shortName/
This way you wouldn't need to care about domain's shortname in the next
patch.
Makes sense. I think I based this on slirpStateDir code. Any reason
it's not using vm->priv->libdir too?
I don't know. But since there are some releases which would store data
under slirpStateDir I don't think we can change this. On daemon
restart (with newer version) the new libvirtd wouldn't see the old dir.
It would need to be done carefully, but it's not out of the question:
There was previously a case where we had been storing the network state
files in an incorrect location (/var/lib/libvirt) (starting in commit
23a090ab92 - libvirt-0.6.0 in 2009), and moved them to a more proper
location (/var/run/libvirt) (commit b9e95491d1 - libvirt 1.2.4 in 2014).
At the time the move was made, we also added a code to
networkStateInitialize() that looked for files in the old location and
moved them to the new location. This code remained in place, at first
doing its job whenever someone upgraded, and then silently doing nothing
for a very long time, until 5 years later when it was removed in commit
43be65a4817f - libvirt 5.1.0. (probably could have been removed much
earlier than that, but it had just been forgotten.)
[PATCH] admin: use g_autofree
From: GAURAV AGRAWAL
Signed-off-by: Gaurav Agrawal
---
src/admin/libvirt-admin.c | 15 +--
1 file changed, 5 insertions(+), 10 deletions(-)
diff --git a/src/admin/libvirt-admin.c b/src/admin/libvirt-admin.c
index 4099a54854..17d0eb39fe 100644
--- a/src/admin/libvirt-admin.c
+++ b/src/admin/libvirt-admin.c
@@ -111,7 +111,7 @@ getSocketPath(virURIPtr uri)
virURIParamPtr param = >params[i];
if (STREQ(param->name, "socket")) {
-VIR_FREE(sock_path);
+g_free(sock_path);
sock_path = g_strdup(param->value);
} else {
virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
@@ -203,11 +203,11 @@ virAdmGetDefaultURI(virConfPtr conf, char **uristr)
virAdmConnectPtr
virAdmConnectOpen(const char *name, unsigned int flags)
{
-char *sock_path = NULL;
+g_autofree char *sock_path = NULL;
char *alias = NULL;
virAdmConnectPtr conn = NULL;
g_autoptr(virConf) conf = NULL;
-char *uristr = NULL;
+g_autofree char *uristr = NULL;
if (virAdmInitialize() < 0)
goto error;
@@ -233,7 +233,7 @@ virAdmConnectOpen(const char *name, unsigned int flags)
goto error;
if (alias) {
-VIR_FREE(uristr);
+g_free(uristr);
uristr = alias;
}
@@ -251,16 +251,11 @@ virAdmConnectOpen(const char *name, unsigned int flags)
if (remoteAdminConnectOpen(conn, flags) < 0)
goto error;
- cleanup:
-VIR_FREE(sock_path);
-VIR_FREE(uristr);
return conn;
error:
virDispatchError(NULL);
-virObjectUnref(conn);
-conn = NULL;
-goto cleanup;
+return NULL;
}
/**
--
2.24.1
[libvirt PATCH] news: Update for libvirt 6.1.0
Signed-off-by: Andrea Bolognani --- I probably won't be able to check my computer between now and the release, so if anyone gets a chance to review the patch in the meantime please feel free to push it as well :) docs/news.xml | 36 1 file changed, 36 insertions(+) diff --git a/docs/news.xml b/docs/news.xml index cdcf450b48..af157887d3 100644 --- a/docs/news.xml +++ b/docs/news.xml @@ -124,6 +124,15 @@ subelement. + + + qemu: Introduce the 'tpm-spapr' TPM model + + + This device, available starting from QEMU 5.0, is limited to + pSeries guests. + + @@ -138,8 +147,35 @@ to rectify the problem. + + + qemu: Support "dies" in CPU topology + + + This CPU topology concept, new in QEMU 4.1.0, sits between the + existing "socket" and "core". + + + + + libxl: Add support for Credit2 scheduler parameters + + + + + lxc: Add support LXC 3 network configuration format + + + + + conf: Do not generate machine names ending with a dash + + + Recent systemd version do not allow them. + + -- 2.24.1
Re: [PATCH] conf: Don't generate machine names with a dot
On 2/28/20 5:22 PM, Michal Privoznik wrote: According to the linked BZ, machined expects either valid hostname or valid FQDN. While in case of multiple dots, a trailing one doesn't violate FQDN, it does violate the rule in case of something simple, like "domain.". But it's safe to remove it in both cases. Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1721804 In the end, I've cloned the bug. The new URL is: https://bugzilla.redhat.com/show_bug.cgi?id=1808499 Fixed locally. Michal
[PATCH] conf: Don't generate machine names with a dot
According to the linked BZ, machined expects either valid
hostname or valid FQDN. While in case of multiple dots, a
trailing one doesn't violate FQDN, it does violate the rule in
case of something simple, like "domain.". But it's safe to remove
it in both cases.
Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1721804
Signed-off-by: Michal Privoznik
---
src/conf/domain_conf.c | 4 ++--
tests/virsystemdtest.c | 1 +
2 files changed, 3 insertions(+), 2 deletions(-)
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index 17867eeece..9371153618 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -30838,8 +30838,8 @@ virDomainMachineNameAppendValid(virBufferPtr buf,
virBufferAddChar(buf, *name);
}
-/* trailing dashes are not allowed */
-virBufferTrimChars(buf, "-");
+/* trailing dashes or dots are not allowed */
+virBufferTrimChars(buf, "-.");
}
#undef HOSTNAME_CHARS
diff --git a/tests/virsystemdtest.c b/tests/virsystemdtest.c
index b7dfd64d06..9847f255ac 100644
--- a/tests/virsystemdtest.c
+++ b/tests/virsystemdtest.c
@@ -744,6 +744,7 @@ mymain(void)
"qemu-100-kstest-network-device-default-httpksc9eed63e-981e-48ec");
TEST_MACHINE("kstest-network-device-default-httpks_(c9eed63e-981e-48ec--cdc-56b3f8c5f678)",
10,
"qemu-10-kstest-network-device-default-httpksc9eed63e-981e-48ec");
+TEST_MACHINE("demo.test.", 11, "qemu-11-demo.test");
# define TESTS_PM_SUPPORT_HELPER(name, function) \
do { \
--
2.24.1
[PATCH 1/2] qemu_shim: Allow other users to enter the root dir
When virt-qemu-run is ran without any root directory specified on
the command line, a temporary directory is made and used instead.
But since we are using g_dir_make_tmp() to create the directory
it is going to have 0700 mode. So even though we create the whole
directory structure under it and label everything, QEMU is very
likely to not have the access. This is because in this case there
is no qemu.conf and thus distro default UID:GID is used to run
QEMU (e.g. qemu:kvm on Fedora). Change the mode of the temporary
directory so that everybody has eXecute permission.
Signed-off-by: Michal Privoznik
---
src/qemu/qemu_shim.c | 6 ++
1 file changed, 6 insertions(+)
diff --git a/src/qemu/qemu_shim.c b/src/qemu/qemu_shim.c
index 5b7840e971..4f06ae952c 100644
--- a/src/qemu/qemu_shim.c
+++ b/src/qemu/qemu_shim.c
@@ -158,6 +158,12 @@ int main(int argc, char **argv)
return 1;
}
tmproot = true;
+
+if (chmod(root, S_IRWXU | S_IXGRP | S_IXOTH) < 0) {
+g_printerr("%s: cannot chown temporary dir: %s\n",
+ argv[0], g_strerror(errno));
+goto cleanup;
+}
}
virFileActivateDirOverrideForProg(argv[0]);
--
2.24.1
[PATCH 2/2] qemu_shim: Ignore SIGPIPE
I've found that if my virtlogd is socket activated but the daemon
doesn't run yet, then the virt-qemu-run is killed right after it
tries to start the domain. The problem is that because the default
setting is to use virtlogd, the domain create code tries to
connect to virtlogd socket, which in turn tries to detect who is
connecting (virNetSocketGetUNIXIdentity()) and as a part of it,
it will try to open /proc/${PID_OF_SHIM}/stat which is denied by
SELinux:
type=AVC msg=audit(1582903501.927:323): avc: denied { search } for \
pid=1210 comm="virtlogd" name="1843" dev="proc" ino=37224 \
scontext=system_u:system_r:virtlogd_t:s0-s0:c0.c1023 \
tcontext=unconfined_u:unconfined_r:unconfined_t:s0-s0:c0.c1023 tclass=dir \
permissive=0
Virtlogd reacts by closing the connection which the shim sees as
SIGPIPE. Since the default response to the signal is Term, we
don't even get to reporting any error nor to removing the
temporary directory.
Signed-off-by: Michal Privoznik
---
src/qemu/qemu_shim.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/src/qemu/qemu_shim.c b/src/qemu/qemu_shim.c
index 4f06ae952c..d8f3902874 100644
--- a/src/qemu/qemu_shim.c
+++ b/src/qemu/qemu_shim.c
@@ -150,6 +150,7 @@ int main(int argc, char **argv)
signal(SIGINT, qemuShimSigShutdown);
signal(SIGQUIT, qemuShimSigShutdown);
signal(SIGHUP, qemuShimSigShutdown);
+signal(SIGPIPE, SIG_IGN);
if (root == NULL) {
if (!(root = g_dir_make_tmp("virt-qemu-run-XX", ))) {
--
2.24.1
[PATCH 0/2] qemu_shim: Two simple fixes
Actually, 2/2 suggests we need to tweak SELinux policy too. Should I file a bug? Michal Prívozník (2): qemu_shim: Allow other users to enter the root dir qemu_shim: Ignore SIGPIPE src/qemu/qemu_shim.c | 7 +++ 1 file changed, 7 insertions(+) -- 2.24.1
Re: [PATCH 3/8] qemu-conf: add dbusStateDir
Hi
On Tue, Feb 25, 2020 at 12:24 PM Michal Privoznik wrote:
>
> On 2/24/20 4:57 PM, Marc-André Lureau wrote:
> > Hi
> >
> > On Thu, Feb 20, 2020 at 10:04 AM Michal Privoznik
> > wrote:
> >>
> >> On 1/14/20 2:46 PM, marcandre.lur...@redhat.com wrote:
> >>> From: Marc-André Lureau
> >>>
> >>> Location of DBus daemon state configuration, socket, pid...
> >>>
> >>> Signed-off-by: Marc-André Lureau
> >>> ---
> >>>src/qemu/qemu_conf.c | 4
> >>>src/qemu/qemu_conf.h | 1 +
> >>>2 files changed, 5 insertions(+)
> >>>
> >>> diff --git a/src/qemu/qemu_conf.c b/src/qemu/qemu_conf.c
> >>> index e1fea390c7..ade12dac6c 100644
> >>> --- a/src/qemu/qemu_conf.c
> >>> +++ b/src/qemu/qemu_conf.c
> >>> @@ -144,6 +144,8 @@ virQEMUDriverConfigPtr virQEMUDriverConfigNew(bool
> >>> privileged)
> >>>
> >>>cfg->cacheDir = g_strdup_printf("%s/cache/libvirt/qemu",
> >>> LOCALSTATEDIR);
> >>>
> >>> +cfg->dbusStateDir = g_strdup_printf("%s/run/libvirt/qemu/dbus",
> >>> LOCALSTATEDIR);
> >>> +
> >>>cfg->libDir = g_strdup_printf("%s/lib/libvirt/qemu",
> >>> LOCALSTATEDIR);
> >>>cfg->saveDir = g_strdup_printf("%s/save", cfg->libDir);
> >>>cfg->snapshotDir = g_strdup_printf("%s/snapshot", cfg->libDir);
> >>> @@ -174,6 +176,7 @@ virQEMUDriverConfigPtr virQEMUDriverConfigNew(bool
> >>> privileged)
> >>>cfg->stateDir = g_strdup_printf("%s/qemu/run", rundir);
> >>>
> >>>cfg->swtpmStateDir = g_strdup_printf("%s/swtpm",
> >>> cfg->stateDir);
> >>> +cfg->dbusStateDir = g_strdup_printf("%s/dbus", cfg->stateDir);
> >>>
> >>>cfg->configBaseDir = virGetUserConfigDirectory();
> >>
> >> Instead of doing practically the same in two branches, you can achieve
> >> the same result with just one line if you put the call just below
> >> cfg->slirpStateDir init.
> >>
> >> However, do we need this to be in a special directory instead of per VM
> >> private directory? The way I implemented PR helper was that the socket
> >> it creates and to which qemu connects is stored under vm->priv->libDir
> >> which is initialized in qemuDomainSetPrivatePaths() to:
> >>
> >> $cfg->libDir/domain-$shortName/
> >>
> >> This way you wouldn't need to care about domain's shortname in the next
> >> patch.
> >
> > Makes sense. I think I based this on slirpStateDir code. Any reason
> > it's not using vm->priv->libdir too?
> >
>
> I don't know. But since there are some releases which would store data
> under slirpStateDir I don't think we can change this. On daemon restart
> (with newer version) the new libvirtd wouldn't see the old dir.
Actually, $cfg->libDir is virQEMUDriverConfigNew ()
cfg->configBaseDir/qemu/lib, so it ends up under ~/.config for users,
and /etc for priviledged/root. A bad place to store transient runtime
data/sockets. You may want to reconsider the paths for pr-helper.
I don't intend to change that, so you could take a look at "[libvirt
PATCH v2 0/9]".
[PATCH 1/8] qemu: capabilities: Add QEMU_CAPS_BLOCKDEV_REOPEN
This capability will be asserted once qemu stabilizes 'blockdev-reopen'.
For now we just add the capability so that we can introduce some code
that will use the reopening call. This will show our willingness to
adopt use of reopen and help qemu developers stabilize it.
Signed-off-by: Peter Krempa
---
src/qemu/qemu_capabilities.c | 1 +
src/qemu/qemu_capabilities.h | 1 +
2 files changed, 2 insertions(+)
diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c
index 5548ffad14..1cb8b697e0 100644
--- a/src/qemu/qemu_capabilities.c
+++ b/src/qemu/qemu_capabilities.c
@@ -564,6 +564,7 @@ VIR_ENUM_IMPL(virQEMUCaps,
/* 355 */
"query-named-block-nodes.flat",
+ "blockdev-reopen",
);
diff --git a/src/qemu/qemu_capabilities.h b/src/qemu/qemu_capabilities.h
index ffb4206723..f7e420a006 100644
--- a/src/qemu/qemu_capabilities.h
+++ b/src/qemu/qemu_capabilities.h
@@ -545,6 +545,7 @@ typedef enum { /* virQEMUCapsFlags grouping marker for
syntax-check */
/* 355 */
QEMU_CAPS_QMP_QUERY_NAMED_BLOCK_NODES_FLAT, /* query-named-block-nodes
supports the 'flat' option */
+QEMU_CAPS_BLOCKDEV_REOPEN, /* 'blockdev-reopen' qmp command is supported */
QEMU_CAPS_LAST /* this must always be the last item */
} virQEMUCapsFlags;
--
2.24.1
[PATCH 7/8] qemuDomainBlockCopyCommon: Record updated flags to block job
For a long time we've masked out VIR_DOMAIN_BLOCK_COPY_SHALLOW if
there's no backing chain for the copied disk to simplify the code.
One of the refacors of caused that we no longer update the 'flags'
variable just the local copies. This was okay until in ccd4228afff I
started storing the job flags in the block job data.
Given that we modify how we call qemu we also should modify @flags so
that the correct value is recorded in the block job data.
Signed-off-by: Peter Krempa
---
src/qemu/qemu_driver.c | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index 76787b6ccc..c3215bccca 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -17915,8 +17915,10 @@ qemuDomainBlockCopyCommon(virDomainObjPtr vm,
}
/* clear the _SHALLOW flag if there is only one layer */
-if (!virStorageSourceHasBacking(disk->src))
+if (!virStorageSourceHasBacking(disk->src)) {
+flags &= ~VIR_DOMAIN_BLOCK_COPY_SHALLOW;
mirror_shallow = false;
+}
if (qemuDomainBlockCopyCommonValidateUserMirrorBackingStore(mirror,
mirror_shallow,
--
2.24.1
[PATCH 8/8] qemu: blockcopy: Allow late opening of the backing chain of a shallow copy
oVirt used a quirk in the pre-blockdev semantics of drive-mirror which
opened the backing chain of the mirror destination only once
'block-job-complete' was called.
Our introduction of blockdev made qemu open the backing chain images
right at the start of the job. This broke oVirt's usage of this API
because they copy the data into the backing chain during the time the
block copy job is running.
Re-introduce late open of the backing chain if qemu supports
blockdev-reopen as we can use that command to install the backing chain
even for an existing image.
Signed-off-by: Peter Krempa
---
src/qemu/qemu_driver.c | 53 +++---
1 file changed, 50 insertions(+), 3 deletions(-)
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index c3215bccca..b746cd92d9 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -17229,6 +17229,7 @@ qemuDomainBlockPivot(virQEMUDriverPtr driver,
qemuBlockJobDataPtr job,
virDomainDiskDefPtr disk)
{
+g_autoptr(qemuBlockStorageSourceChainData) chainattachdata = NULL;
int ret = -1;
qemuDomainObjPrivatePtr priv = vm->privateData;
bool blockdev = virQEMUCapsGet(priv->qemuCaps, QEMU_CAPS_BLOCKDEV);
@@ -17263,6 +17264,7 @@ qemuDomainBlockPivot(virQEMUDriverPtr driver,
if (blockdev && !job->jobflagsmissing) {
g_autoptr(virHashTable) blockNamedNodeData = NULL;
bool shallow = job->jobflags & VIR_DOMAIN_BLOCK_COPY_SHALLOW;
+bool reuse = job->jobflags & VIR_DOMAIN_BLOCK_COPY_REUSE_EXT;
if (!(blockNamedNodeData = qemuBlockGetNamedNodeData(vm,
QEMU_ASYNC_JOB_NONE)))
return -1;
@@ -17271,6 +17273,17 @@ qemuDomainBlockPivot(virQEMUDriverPtr driver,
blockNamedNodeData,
shallow, ) < 0)
return -1;
+
+/* Open and install the backing chain of 'mirror' late if we
support
+ * blockdev-reopen. This is to appease oVirt that wants to copy
+ * data into the backing chain while the top image is being copied
+ * shallow */
+if (reuse && shallow &&
+virQEMUCapsGet(priv->qemuCaps, QEMU_CAPS_BLOCKDEV_REOPEN)) {
+if (!(chainattachdata =
qemuBuildStorageSourceChainAttachPrepareBlockdev(disk->mirror->backingStore,
+
priv->qemuCaps)))
+return -1;
+}
}
break;
@@ -17278,6 +17291,23 @@ qemuDomainBlockPivot(virQEMUDriverPtr driver,
break;
}
+if (chainattachdata) {
+int rc;
+
+qemuDomainObjEnterMonitor(driver, vm);
+rc = qemuBlockStorageSourceChainAttach(priv->mon, chainattachdata);
+if (qemuDomainObjExitMonitor(driver, vm) < 0 || rc < 0)
+return -1;
+
+if (qemuBlockReopenFormat(vm, disk->mirror, QEMU_ASYNC_JOB_NONE) < 0) {
+qemuDomainObjEnterMonitor(driver, vm);
+qemuBlockStorageSourceChainDetach(priv->mon, chainattachdata);
+if (qemuDomainObjExitMonitor(driver, vm) < 0)
+return -1;
+return -1;
+}
+}
+
qemuDomainObjEnterMonitor(driver, vm);
if (blockdev) {
int rc = 0;
@@ -18027,9 +18057,26 @@ qemuDomainBlockCopyCommon(virDomainObjPtr vm,
if (blockdev) {
if (mirror_reuse) {
-if (!(data =
qemuBuildStorageSourceChainAttachPrepareBlockdev(mirror,
-
priv->qemuCaps)))
-goto endjob;
+/* oVirt depended on late-backing-chain-opening semantics the old
+ * qemu command had to copy the backing chain data while the top
+ * level is being copied. To restore this semantics if
+ * blockdev-reopen is supported defer opening of the backing chain
+ * of 'mirror' to the pivot step */
+if (virQEMUCapsGet(priv->qemuCaps, QEMU_CAPS_BLOCKDEV_REOPEN)) {
+g_autoptr(virStorageSource) terminator = virStorageSourceNew();
+
+if (!terminator)
+goto endjob;
+
+if (!(data =
qemuBuildStorageSourceChainAttachPrepareBlockdevTop(mirror,
+
terminator,
+
priv->qemuCaps)))
+goto endjob;
+} else {
+if (!(data =
qemuBuildStorageSourceChainAttachPrepareBlockdev(mirror,
+
priv->qemuCaps)))
+goto endjob;
+}
} else {
if (!(blockNamedNodeData =
[PATCH 4/8] qemuCheckpointDiscardBitmaps: Reopen images for bitmap modifications
Qemu's bitmap APIs don't reopen the appropriate images read-write for
modification. It's libvirt's duty to reopen them via blockdev-reopen
if we wish to modify the bitmaps.
Use the new helpers to reopen the images for bitmap manipulation.
Signed-off-by: Peter Krempa
---
src/qemu/qemu_checkpoint.c | 4
1 file changed, 4 insertions(+)
diff --git a/src/qemu/qemu_checkpoint.c b/src/qemu/qemu_checkpoint.c
index a387e7dfe7..32b0ab0faf 100644
--- a/src/qemu/qemu_checkpoint.c
+++ b/src/qemu/qemu_checkpoint.c
@@ -299,6 +299,9 @@ qemuCheckpointDiscardBitmaps(virDomainObjPtr vm,
if (qemuDomainStorageSourceAccessAllow(driver, vm, src, false, false)
< 0)
goto relabel;
+if (qemuBlockReopenReadWrite(vm, src, QEMU_ASYNC_JOB_NONE) < 0)
+goto relabel;
+
relabelimages = g_slist_prepend(relabelimages, src);
}
@@ -311,6 +314,7 @@ qemuCheckpointDiscardBitmaps(virDomainObjPtr vm,
for (next = relabelimages; next; next = next->next) {
virStorageSourcePtr src = next->data;
+ignore_value(qemuBlockReopenReadOnly(vm, src, QEMU_ASYNC_JOB_NONE));
ignore_value(qemuDomainStorageSourceAccessAllow(driver, vm, src, true,
false));
}
--
2.24.1
[PATCH 6/8] qemuDomainBlockPivot: Move check prior to executing the pivot steps
Move the check whether the job is already synchronised to the beginning
of the function so that we don't try to do some of the steps necessary
for pivoting prior to actually wanting to pivot.
Signed-off-by: Peter Krempa
---
src/qemu/qemu_driver.c | 14 +++---
1 file changed, 7 insertions(+), 7 deletions(-)
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index 35ade1ef37..76787b6ccc 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -17234,6 +17234,13 @@ qemuDomainBlockPivot(virQEMUDriverPtr driver,
bool blockdev = virQEMUCapsGet(priv->qemuCaps, QEMU_CAPS_BLOCKDEV);
g_autoptr(virJSONValue) actions = NULL;
+if (job->state != QEMU_BLOCKJOB_STATE_READY) {
+virReportError(VIR_ERR_BLOCK_COPY_ACTIVE,
+ _("block job '%s' not ready for pivot yet"),
+ job->name);
+return -1;
+}
+
switch ((qemuBlockJobType) job->type) {
case QEMU_BLOCKJOB_TYPE_NONE:
case QEMU_BLOCKJOB_TYPE_LAST:
@@ -17271,13 +17278,6 @@ qemuDomainBlockPivot(virQEMUDriverPtr driver,
break;
}
-if (job->state != QEMU_BLOCKJOB_STATE_READY) {
-virReportError(VIR_ERR_BLOCK_COPY_ACTIVE,
- _("block job '%s' not ready for pivot yet"),
- job->name);
-return -1;
-}
-
qemuDomainObjEnterMonitor(driver, vm);
if (blockdev) {
int rc = 0;
--
2.24.1
[PATCH 5/8] qemuCheckpointDiscardBitmaps: Use correct field for checkpoint bitmap name
The code deleting checkpoints needs the name of the parent checkpoint's disk's bitmap but was using the disk alias instead. This would create wrong bitmaps after deleting some checkpoints. Signed-off-by: Peter Krempa --- src/qemu/qemu_checkpoint.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/qemu/qemu_checkpoint.c b/src/qemu/qemu_checkpoint.c index 32b0ab0faf..378f6c147e 100644 --- a/src/qemu/qemu_checkpoint.c +++ b/src/qemu/qemu_checkpoint.c @@ -283,7 +283,7 @@ qemuCheckpointDiscardBitmaps(virDomainObjPtr vm, * ancestor. */ if ((parentchkdisk = qemuCheckpointFindActiveDiskInParent(vm, parent, chkdisk->name))) -parentbitmap = parentchkdisk->name; +parentbitmap = parentchkdisk->bitmap; if (qemuCheckpointDiscardDiskBitmaps(domdisk->src, blockNamedNodeData, chkdisk->bitmap, parentbitmap, -- 2.24.1
[PATCH 0/8] qemu: Show willingness to use blockdev-reopen
To break the chicken and egg problem loop between qemu and libvirt in using new features introduce experimental support for blockdev-reopen (or actually x-blockdev-reopen for the time being). This patchset adds QEMU_CAPS_BLOCKDEV_REOPEN capability which is currently not asserted until qemu stabilizes the blockdev-reopen interface but implements all the handlers to use it. This is a similar approach we used to add all of the bits required to use -blockdev with qemu. To show it's usefullnes two real problems are addressed using reopening: - Checkpoint deletion in backing chain, where we need to reopen the read-only backing images to allow modification of bitmaps. Using this approach will prevent qemu from having to introduce yet another ad-hoc interface to deal with the bitmaps. (note that checkpoints are also experimental themselves since they are part of the not-yet-finished incremental backup feature) - Late open of backing files for virDomainBlockCopy oVirt abuses a quirk in the old handling of block-copy when drive-mirror is used as qemu opens the backing images of the destination of the copy only once block-job-complete is called. Without blockdev-reopen it's impossible to replicate the old semantics as we need to install a backing file for the mirror copy and that is possible only using blockdev-reopen. (this change will stay disabled until blockdev-reopen is stabilized) There are a few other problems which this will deal with mostly related to bitmap handling which would also require ad-hoc qemu functionality otherwise. Since we have an existing interface we can show we are willing to use it to prevent wasting more engieering on qemu's side on partial solutions. This patchset applies on top of: https://www.redhat.com/archives/libvir-list/2020-February/msg01062.html It can be fetched from my repo: git fetch https://gitlab.com/pipo.sk/libvirt.git reopen-impl https://gitlab.com/pipo.sk/libvirt/-/commits/reopen-impl Note the above branch contains also patches which enable the feature and also enable incremental backup to facilitate simple testing without the need to use the qemu namespace. Successful use requires the following qemu patches: https://lists.gnu.org/archive/html/qemu-block/2020-02/msg01423.html https://lists.gnu.org/archive/html/qemu-block/2020-02/msg01467.html A qemu repo containing the above patches and patch to enable the detection done in my private brnch mentioned above can be fetched at: git fetch https://gitlab.com/pipo.sk/qemu.git bitmap-reopen https://gitlab.com/pipo.sk/qemu/-/commits/bitmap-reopen Peter Krempa (8): qemu: capabilities: Add QEMU_CAPS_BLOCKDEV_REOPEN qemu: monitor: Add handler for blockdev-reopen qemu: block: implement helpers for blockdev-reopen qemuCheckpointDiscardBitmaps: Reopen images for bitmap modifications qemuCheckpointDiscardBitmaps: Use correct field for checkpoint bitmap name qemuDomainBlockPivot: Move check prior to executing the pivot steps qemuDomainBlockCopyCommon: Record updated flags to block job qemu: blockcopy: Allow late opening of the backing chain of a shallow copy src/qemu/qemu_block.c| 121 +++ src/qemu/qemu_block.h| 14 src/qemu/qemu_capabilities.c | 1 + src/qemu/qemu_capabilities.h | 1 + src/qemu/qemu_checkpoint.c | 6 +- src/qemu/qemu_driver.c | 67 --- src/qemu/qemu_monitor.c | 13 src/qemu/qemu_monitor.h | 3 + src/qemu/qemu_monitor_json.c | 21 ++ src/qemu/qemu_monitor_json.h | 4 ++ 10 files changed, 241 insertions(+), 10 deletions(-) -- 2.24.1
[PATCH 3/8] qemu: block: implement helpers for blockdev-reopen
Introduce a set of helpers to call blockdev-reopen in certain scenarios
Libvirt will use the QMP command to turn certain members of the backing
chain read-write for bitmap manipulation and we'll also want to use it
to replace/install the backing chain of a qcow2 format node.
Signed-off-by: Peter Krempa
---
src/qemu/qemu_block.c | 121 ++
src/qemu/qemu_block.h | 14 +
2 files changed, 135 insertions(+)
diff --git a/src/qemu/qemu_block.c b/src/qemu/qemu_block.c
index 152c73f1bf..441b8ec07b 100644
--- a/src/qemu/qemu_block.c
+++ b/src/qemu/qemu_block.c
@@ -2960,3 +2960,124 @@ qemuBlockBitmapsHandleBlockcopy(virStorageSourcePtr src,
return 0;
}
+
+
+/**
+ * qemuBlockReopen:
+ * @vm: domain object
+ * @props: JSON object used as argument of 'blockdev-reopen'
+ * @asyncJob: qemu async job type
+ *
+ * Monitor interaction to call 'blockdev-reopen' @props is consumed on
successful
+ * call and set to NULL if consumed.
+ */
+static int
+qemuBlockReopen(virDomainObjPtr vm,
+virJSONValuePtr *props,
+qemuDomainAsyncJob asyncJob)
+{
+qemuDomainObjPrivatePtr priv = vm->privateData;
+virQEMUDriverPtr driver = priv->driver;
+int rc;
+
+if (qemuDomainObjEnterMonitorAsync(driver, vm, asyncJob) < 0)
+return -1;
+
+rc = qemuMonitorBlockdevReopen(priv->mon, props);
+
+if (qemuDomainObjExitMonitor(driver, vm) < 0 || rc < 0)
+return -1;
+
+return 0;
+}
+
+
+/**
+ * qemuBlockReopenFormat:
+ * @vm: domain object
+ * @src: storage source to reopen
+ * @asyncJob: qemu async job type
+ *
+ * Invokes the 'blockdev-reopen' command on the format layer of @src. This
means
+ * that @src must be already properly configured for the desired outcome. The
+ * nodenames of @src are used to identify the specific image in qemu. This
+ * function enters the monitor.
+ */
+int
+qemuBlockReopenFormat(virDomainObjPtr vm,
+ virStorageSourcePtr src,
+ qemuDomainAsyncJob asyncJob)
+{
+g_autoptr(virJSONValue) reopenprops = NULL;
+
+/* If we are lacking the object here, qemu might have opened an image with
+ * a node name unknown to us */
+if (!src->backingStore) {
+virReportError(VIR_ERR_OPERATION_UNSUPPORTED, "%s",
+ _("can't reopen image with unknown presence of backing
store"));
+return -1;
+}
+
+if (!(reopenprops = qemuBlockStorageSourceGetBlockdevProps(src,
src->backingStore)))
+return -1;
+
+return qemuBlockReopen(vm, , asyncJob);
+}
+
+
+/**
+ * qemuBlockReopenReadWrite:
+ * @vm: domain object
+ * @src: storage source to reopen
+ * @asyncJob: qemu async job type
+ *
+ * Wrapper that reopens @src read-write. We currently depend on qemu
+ * reopening the storage with 'auto-read-only' enabled for us.
+ * After successful reopen @src's 'readonly' flag is modified. Does nothing
+ * if @src is already read-write.
+ */
+int
+qemuBlockReopenReadWrite(virDomainObjPtr vm,
+ virStorageSourcePtr src,
+ qemuDomainAsyncJob asyncJob)
+{
+if (!src->readonly)
+return 0;
+
+src->readonly = false;
+if (qemuBlockReopenFormat(vm, src, asyncJob) < 0) {
+src->readonly = true;
+return -1;
+}
+
+return 0;
+}
+
+
+/**
+ * qemuBlockReopenReadOnly:
+ * @vm: domain object
+ * @src: storage source to reopen
+ * @asyncJob: qemu async job type
+ *
+ * Wrapper that reopens @src read-only. We currently depend on qemu
+ * reopening the storage with 'auto-read-only' enabled for us.
+ * After successful reopen @src's 'readonly' flag is modified. Does nothing
+ * if @src is already read-only.
+ */
+int
+qemuBlockReopenReadOnly(virDomainObjPtr vm,
+ virStorageSourcePtr src,
+ qemuDomainAsyncJob asyncJob)
+{
+if (src->readonly)
+return 0;
+
+src->readonly = true;
+if (qemuBlockReopenFormat(vm, src, asyncJob) < 0) {
+src->readonly = false;
+return -1;
+}
+
+return 0;
+}
diff --git a/src/qemu/qemu_block.h b/src/qemu/qemu_block.h
index eab0128d5d..b3d7d0f876 100644
--- a/src/qemu/qemu_block.h
+++ b/src/qemu/qemu_block.h
@@ -228,3 +228,17 @@ qemuBlockBitmapsHandleBlockcopy(virStorageSourcePtr src,
virHashTablePtr blockNamedNodeData,
bool shallow,
virJSONValuePtr *actions);
+
+int
+qemuBlockReopenFormat(virDomainObjPtr vm,
+ virStorageSourcePtr src,
+ qemuDomainAsyncJob asyncJob);
+
+int
+qemuBlockReopenReadWrite(virDomainObjPtr vm,
+ virStorageSourcePtr src,
+ qemuDomainAsyncJob asyncJob);
+int
+qemuBlockReopenReadOnly(virDomainObjPtr vm,
+virStorageSourcePtr src,
+qemuDomainAsyncJob asyncJob);
--
2.24.1
[PATCH 2/8] qemu: monitor: Add handler for blockdev-reopen
Introduce the monitor code for using blockdev-reopen. For now we'll use
x-blockdev-reopen so that the interactions between qemu and libvirt
can be tested with the existing code.
Since the usage will be guarded by the for-now unasserted capability
we'll be able to change the called command when the command will be
stabilized.
Signed-off-by: Peter Krempa
---
src/qemu/qemu_monitor.c | 13 +
src/qemu/qemu_monitor.h | 3 +++
src/qemu/qemu_monitor_json.c | 21 +
src/qemu/qemu_monitor_json.h | 4
4 files changed, 41 insertions(+)
diff --git a/src/qemu/qemu_monitor.c b/src/qemu/qemu_monitor.c
index c2a61ec587..3fab31aa38 100644
--- a/src/qemu/qemu_monitor.c
+++ b/src/qemu/qemu_monitor.c
@@ -4378,6 +4378,19 @@ qemuMonitorBlockdevAdd(qemuMonitorPtr mon,
}
+int
+qemuMonitorBlockdevReopen(qemuMonitorPtr mon,
+ virJSONValuePtr *props)
+{
+VIR_DEBUG("props=%p (node-name=%s)", *props,
+ NULLSTR(virJSONValueObjectGetString(*props, "node-name")));
+
+QEMU_CHECK_MONITOR(mon);
+
+return qemuMonitorJSONBlockdevReopen(mon, props);
+}
+
+
int
qemuMonitorBlockdevDel(qemuMonitorPtr mon,
const char *nodename)
diff --git a/src/qemu/qemu_monitor.h b/src/qemu/qemu_monitor.h
index 4e06447ffa..23b8d0a650 100644
--- a/src/qemu/qemu_monitor.h
+++ b/src/qemu/qemu_monitor.h
@@ -1324,6 +1324,9 @@ int qemuMonitorBlockdevCreate(qemuMonitorPtr mon,
int qemuMonitorBlockdevAdd(qemuMonitorPtr mon,
virJSONValuePtr *props);
+int qemuMonitorBlockdevReopen(qemuMonitorPtr mon,
+ virJSONValuePtr *props);
+
int qemuMonitorBlockdevDel(qemuMonitorPtr mon,
const char *nodename);
diff --git a/src/qemu/qemu_monitor_json.c b/src/qemu/qemu_monitor_json.c
index 451e1afef5..0f968b7d36 100644
--- a/src/qemu/qemu_monitor_json.c
+++ b/src/qemu/qemu_monitor_json.c
@@ -8837,6 +8837,27 @@ qemuMonitorJSONBlockdevAdd(qemuMonitorPtr mon,
}
+int
+qemuMonitorJSONBlockdevReopen(qemuMonitorPtr mon,
+ virJSONValuePtr *props)
+{
+g_autoptr(virJSONValue) cmd = NULL;
+g_autoptr(virJSONValue) reply = NULL;
+virJSONValuePtr pr = g_steal_pointer(props);
+
+if (!(cmd = qemuMonitorJSONMakeCommandInternal("x-blockdev-reopen", pr)))
+return -1;
+
+if (qemuMonitorJSONCommand(mon, cmd, ) < 0)
+return -1;
+
+if (qemuMonitorJSONCheckError(cmd, reply) < 0)
+return -1;
+
+return 0;
+}
+
+
int
qemuMonitorJSONBlockdevDel(qemuMonitorPtr mon,
const char *nodename)
diff --git a/src/qemu/qemu_monitor_json.h b/src/qemu/qemu_monitor_json.h
index ed48600b82..5b3bb295eb 100644
--- a/src/qemu/qemu_monitor_json.h
+++ b/src/qemu/qemu_monitor_json.h
@@ -602,6 +602,10 @@ int qemuMonitorJSONBlockdevAdd(qemuMonitorPtr mon,
virJSONValuePtr *props)
ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2);
+int qemuMonitorJSONBlockdevReopen(qemuMonitorPtr mon,
+ virJSONValuePtr *props)
+ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2);
+
int qemuMonitorJSONBlockdevDel(qemuMonitorPtr mon,
const char *nodename)
ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2);
--
2.24.1
Re: [PATCH 2/2] qemu: Tell secdrivers which images are top parent
On Thu, Feb 27, 2020 at 13:07:36 +0100, Michal Privoznik wrote:
> When preparing images for block jobs we modify their seclabels so
> that QEMU can open them. However, as mentioned in the previous
> commit, secdrivers base some it their decisions whether the image
> they are working on is top parent or not. Fortunately, in places
top of the backing chain
> where we call secdrivers we know this and the information can be
> passed to secdrivers.
>
> This fixes the problem described in the linked bugzilla. The
That's what patches usually do. Either state the problem or omit this
sentece.
> problem is the following: after the first blockcommit from the
> base to one of the parents the XATTRs on the base image are not
> cleared and therefore the second attempt to do another
Oh you do state the problem. So just omit that sentence.
> blockcommit fails. This is caused by blockcommit code calling
> qemuSecuritySetImageLabel() over the base image and never calling
> the corresponding qemuSecurityRestoreImageLabel(). A naive fix
> would be to call the restore function. But this is not possible,
> because that would deny QEMU the access to the base image.
Well this kind of misleads. We want to modify the security label so that
the VM has read-write or read-only access. The security label is then
corrected by the call to another qemuSecuritySetImageLabel. You then
correctly mention that using qemuSecurityRestoreImageLabel would cut the
access to the image.
> Fortunately, we can use the fact that seclabels are remembered
> only for the top parent and not for the rest of the backing
'top of backing chain'
> chain. And thanks to the previous commit we can tell secdrivers
> which images are top parents.
>
> Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1803551
>
> Signed-off-by: Michal Privoznik
> ---
> src/qemu/qemu_backup.c | 4 ++--
> src/qemu/qemu_blockjob.c | 6 --
> src/qemu/qemu_checkpoint.c | 6 --
> src/qemu/qemu_domain.c | 15 +--
> src/qemu/qemu_domain.h | 3 ++-
> src/qemu/qemu_driver.c | 15 ++-
> src/qemu/qemu_process.c| 2 +-
> src/qemu/qemu_security.c | 6 +-
> src/qemu/qemu_security.h | 3 ++-
> 9 files changed, 43 insertions(+), 17 deletions(-)
[...]
> diff --git a/src/qemu/qemu_blockjob.c b/src/qemu/qemu_blockjob.c
> index 71df0d1ab2..9db1b71a3e 100644
> --- a/src/qemu/qemu_blockjob.c
> +++ b/src/qemu/qemu_blockjob.c
> @@ -1105,9 +1105,11 @@
> qemuBlockJobProcessEventCompletedCommit(virQEMUDriverPtr driver,
> return;
>
> /* revert access to images */
> -qemuDomainStorageSourceAccessAllow(driver, vm, job->data.commit.base,
> true, false);
> +qemuDomainStorageSourceAccessAllow(driver, vm, job->data.commit.base,
> + true, false, false);
> if (job->data.commit.topparent != job->disk->src)
> -qemuDomainStorageSourceAccessAllow(driver, vm,
> job->data.commit.topparent, true, false);
> +qemuDomainStorageSourceAccessAllow(driver, vm,
> job->data.commit.topparent,
> + true, false, false);
Here you see the misleading name. This is called to relabel an image
called 'topparent' but yet set the 'topparent' flag to false.
> baseparent->backingStore = NULL;
> job->data.commit.topparent->backingStore = job->data.commit.base;
[...]
> diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c
> index 3dfa71650d..32e8220d98 100644
> --- a/src/qemu/qemu_domain.c
> +++ b/src/qemu/qemu_domain.c
> @@ -11589,6 +11589,8 @@ typedef enum {
> QEMU_DOMAIN_STORAGE_SOURCE_ACCESS_SKIP_REVOKE = 1 << 4,
> /* VM already has access to the source and we are just modifying it */
> QEMU_DOMAIN_STORAGE_SOURCE_ACCESS_MODIFY_ACCESS = 1 << 5,
> +/* whether the image is top parent of backing chain */
> +QEMU_DOMAIN_STORAGE_SOURCE_ACCESS_TOP_PARENT = 1 << 6,
whether the image is the top image of the backing chain (e.g. disk source)
QEMU_DOMAIN_STORAGE_SOURCE_ACCESS_CHAIN_TOP
> } qemuDomainStorageSourceAccessFlags;
>
>
> @@ -11817,6 +11820,7 @@ qemuDomainStorageSourceAccessRevoke(virQEMUDriverPtr
> driver,
> * @elem: source structure to set access for
> * @readonly: setup read-only access if true
> * @newSource: @elem describes a storage source which @vm can't access yet
> + * @topparent: @elem is top parent of backing chain
> *
> * Allow a VM access to a single element of a disk backing chain; this helper
> * ensures that the lock manager, cgroup device controller, and security
> manager
> @@ -11824,13 +11828,17 @@
> qemuDomainStorageSourceAccessRevoke(virQEMUDriverPtr driver,
> *
> * When modifying permissions of @elem which @vm can already access (is in
> the
> * backing chain) @newSource needs to be set to false.
> + *
> + * When the @elem is top parent of a backing chain, then @topparent must be
> + * true, otherwise it must be false.
You want to specify
