Re: [libvirt] [PATCH] qemu: snapshot: Fix libvirtd crash in snapshot-revert
On Fri, Dec 13, 2019 at 15:47:36 +0800, Lin Ma wrote:
> When reverting a running domain to a snapshot(active state), We need to
> use the FORCE flag for snapshot-revert if current domain configuration
> is different from the target domain configuration, and this will start a
> new qemu instance for the target domain.
>
> In this situation, if there is existing connection to the domain, say
> Spice or VNC through virt-manager, Then the libvirtd would crash during
> snapshot revert because: Both of snapshot revert worker and new worker
> job 'remoteDispatchDomainOpenGraphicsFd' are waiting for mon->msg->finished
> in qemuMonitorSend(), We know if IO process resulted in an error with a
> message, Libvirtd main thread calls qemuMonitorIO() to wakeup the waiter.
> Then mon->msg will be set to NULL in qemuMonitorSend() once the worker
> GraphicsFD is woken up, which causes snapshot revert worker dereferences
> this null pointer.
[]
> Signed-off-by: Lin Ma
> ---
> src/qemu/qemu_monitor.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/src/qemu/qemu_monitor.c b/src/qemu/qemu_monitor.c
> index ea3e62dc8e..a8344e698b 100644
> --- a/src/qemu/qemu_monitor.c
> +++ b/src/qemu/qemu_monitor.c
> @@ -994,7 +994,7 @@ qemuMonitorSend(qemuMonitorPtr mon,
>"mon=%p msg=%s fd=%d",
>mon, mon->msg->txBuffer, mon->msg->txFD);
>
> -while (!mon->msg->finished) {
> +while (mon->msg && !mon->msg->finished) {
This fixes only the symptom. The actual problem is in handling of our
job state when restarting the qemu process:
Please see the following patches which aim to fix the same problem:
https://www.redhat.com/archives/libvir-list/2019-December/msg00663.html
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH] qemu: snapshot: Fix libvirtd crash in snapshot-revert
When reverting a running domain to a snapshot(active state), We need to
use the FORCE flag for snapshot-revert if current domain configuration
is different from the target domain configuration, and this will start a
new qemu instance for the target domain.
In this situation, if there is existing connection to the domain, say
Spice or VNC through virt-manager, Then the libvirtd would crash during
snapshot revert because: Both of snapshot revert worker and new worker
job 'remoteDispatchDomainOpenGraphicsFd' are waiting for mon->msg->finished
in qemuMonitorSend(), We know if IO process resulted in an error with a
message, Libvirtd main thread calls qemuMonitorIO() to wakeup the waiter.
Then mon->msg will be set to NULL in qemuMonitorSend() once the worker
GraphicsFD is woken up, which causes snapshot revert worker dereferences
this null pointer.
Not sure whether this scenario makes sense, But at least the libvirtd
should not crash, So fix it.
Thread 6 "libvirtd" hit Breakpoint 1, qemuMonitorSend
987 if (mon->lastError.code != VIR_ERR_OK) {
(gdb) bt
#0 in qemuMonitorSend
#1 in qemuMonitorJSONCommandWithFd
#2 in qemuMonitorJSONSendFileHandle
#3 in qemuMonitorSendFileHandle
#4 in qemuMonitorOpenGraphics
#5 in qemuDomainOpenGraphicsFD
#6 in virDomainOpenGraphicsFD
#7 in remoteDispatchDomainOpenGraphicsFd
#8 in remoteDispatchDomainOpenGraphicsFdHelper
#9 in virNetServerProgramDispatchCall
#10 in virNetServerProgramDispatch
#11 in virNetServerProcessMsg
#12 in virNetServerHandleJob
#13 in virThreadPoolWorker
#14 in virThreadHelper
#15 in start_thread
#16 in clone
(gdb) c
Continuing.
Thread 2 "libvirtd" received signal SIGSEGV, Segmentation fault.
[Switching to Thread 0x7fbd20fe2700 (LWP 19014)]
0x7fbd1c3be838 in qemuMonitorSend (mon=mon@entry=0x7fbd1804a940,
msg=msg@entry=0x7fbd20fe15b0)
at ../../src/qemu/qemu_monitor.c:1001
1001while (!mon->msg->finished) {
(gdb) bt
#0 in qemuMonitorSend
#1 in qemuMonitorJSONCommandWithFd
#2 in qemuMonitorJSONCommand
#3 in qemuMonitorJSONGetChardevInfo
#4 in qemuMonitorGetChardevInfo
#5 in qemuProcessWaitForMonitor
#6 in qemuProcessLaunch
#7 in qemuProcessStart
#8 in qemuDomainRevertToSnapshot
#9 in virDomainRevertToSnapshot
#10 in remoteDispatchDomainRevertToSnapshot
#11 in remoteDispatchDomainRevertToSnapshotHelper
#12 in virNetServerProgramDispatchCall
#13 in virNetServerProgramDispatch
#14 in virNetServerProcessMsg
#15 in virNetServerHandleJob
#16 in virThreadPoolWorker
#17 in virThreadHelper
#18 in start_thread
#19 in clone
Signed-off-by: Lin Ma
---
src/qemu/qemu_monitor.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/src/qemu/qemu_monitor.c b/src/qemu/qemu_monitor.c
index ea3e62dc8e..a8344e698b 100644
--- a/src/qemu/qemu_monitor.c
+++ b/src/qemu/qemu_monitor.c
@@ -994,7 +994,7 @@ qemuMonitorSend(qemuMonitorPtr mon,
"mon=%p msg=%s fd=%d",
mon, mon->msg->txBuffer, mon->msg->txFD);
-while (!mon->msg->finished) {
+while (mon->msg && !mon->msg->finished) {
if (virCondWait(&mon->notify, &mon->parent.lock) < 0) {
virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
_("Unable to wait on monitor condition"));
--
2.23.0
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 01/19] virsh: Add QMP command wrapping for 'qemu-monitor-command'
On Thu, Dec 12, 2019 at 12:16:11 -0600, Eric Blake wrote:
> On 12/12/19 11:18 AM, Peter Krempa wrote:
> > Issuing simple QMP commands is pain as they need to be wrapped by the
> > JSON wrapper:
> >
> > { "execute": "COMMAND" }
> >
> > and optionally also:
> >
> > { "execute": "COMMAND", "arguments":...}
> >
> > For simple commands without arguments we can add syntax sugar to virsh
> > which allows simple usage of QMP and additionally prepares also for
> > passing through of the 'arguments' section.
>
> I'd give an example of the new syntax in the commit message:
>
> virsh qemu-monitor-command domain --qmp COMMAND '{ARGUMENTS...}'
>
> as shorthand for
>
> virsh qemu-monitor-command domain '"execute":"COMMAND",
> "arguments":{ARGUMENTS...}}'
>
> But the sugar is indeed nice (one less layer of {} JSON).
>
> >
> > Signed-off-by: Peter Krempa
> > ---
> > docs/manpages/virsh.rst | 24 +---
> > tools/virsh-domain.c| 32 +---
> > 2 files changed, 46 insertions(+), 10 deletions(-)
> >
>
> > -virBufferTrim(&buf, " ", -1);
> > +if ((opt = vshCommandOptArgv(ctl, cmd, opt)))
> > +command = opt->data;
> > +if ((opt = vshCommandOptArgv(ctl, cmd, opt)))
> > +arguments = opt->data;
> > +
> > +if (!command || (arguments && vshCommandOptArgv(ctl, cmd, opt))) {
> > +vshError(ctl, "%s", _("-qmp option requires 1 or 2
> > arguments"));
> > +return false;
>
> Should we allow concatenation and/or magic behavior based on whether the
> second argument starts with '{'? For example,
>
> virsh qemu-monitor-command --qmp COMMAND key1=1 'key2="str"'
>
> could be shorthand for
>
> virsh qemu-monitor-command '{"execute":"COMMAND", "arguments":{"key1":1,
> "key2":"str"}}'
I thought about this originally, but you also need a way to expose the
'null' field and booleans. Also the necessary double quoting on strings
isn't exactly user friendly given that it's the most common type of
argument.
Also all of the above can only support creating a flat object as
argument so we still need a way to do nested objects or arrays. I agree
though that it's the most common case so any nested or non-default one
can stay JSON-only.
Thus I'll be okay doing this, but I thin we should do something else for
the strings since shell quoting is often confusing to users.
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 01/19] virsh: Add QMP command wrapping for 'qemu-monitor-command'
On Thu, Dec 12, 2019 at 19:30:27 +0100, Michal Privoznik wrote:
> On 12/12/19 7:16 PM, Eric Blake wrote:
> > On 12/12/19 11:18 AM, Peter Krempa wrote:
> > > Issuing simple QMP commands is pain as they need to be wrapped by the
> > > JSON wrapper:
> > >
> > > { "execute": "COMMAND" }
> > >
> > > and optionally also:
> > >
> > > { "execute": "COMMAND", "arguments":...}
> > >
> > > For simple commands without arguments we can add syntax sugar to virsh
> > > which allows simple usage of QMP and additionally prepares also for
> > > passing through of the 'arguments' section.
> >
> > I'd give an example of the new syntax in the commit message:
> >
> > virsh qemu-monitor-command domain --qmp COMMAND '{ARGUMENTS...}'
> >
> > as shorthand for
> >
> > virsh qemu-monitor-command domain '"execute":"COMMAND",
> > "arguments":{ARGUMENTS...}}'
> >
> > But the sugar is indeed nice (one less layer of {} JSON).
>
> Since we won't use HMP to talk to qemu ever (even the small set of HMP
> commands we have are wrapped inside QMP once being sent down the wire), can
Specifically you already must use --hmp if you ever want to use HMP.
> we not use --qmp flag at all? Just look if there's "execute" in the user's
> input and if not add it there. For instance:
>
> virsh qemu-monitor-command query-machines
>
> will expand to
>
> {"execute":"query-machines"}
At first I wanted to argue that I'd like to support passing raw
unmodified commands to qemu, but in fact libvirt itself parses the
string as JSON so that it can be re-wrapped with the monitor sequence
field, so you have to pass in JSON anyways.
Thus I'm okay with dropping the flag and deciding on whether the opening
'{' is present on input.
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v3 0/6] PCI hostdev partial assignment support
On 12/11/19 8:49 PM, Cole Robinson wrote: On 11/26/19 8:33 AM, Daniel Henrique Barboza wrote: changes from previous version [1]: [...] One comment Daniel Henrique Barboza (6): Introducing new address type='unassigned' for PCI hostdevs qemu: handle unassigned PCI hostdevs in command line and alias Is there a specific reason to skip alias assign, beside it not being needed for the command line? If it doesn't hurt, maybe we just keep it. I don't have a strong argument for it though. If no one says anything I'll leave it as is The reason why I was skipping aliases was cosmetic due to QEMU command line. I find it a bit odd that, in a scenario with 4 functions where function 1 is unassigned, QEMU cmd line would have hostdev0, hostdev2 and hostdev3, because 'hostdev1' alias got assigned to the unassigned function '1' I don't have any strong feelings about this though. We can keep giving aliases or all functions, regardless of whether they are being assigned to the guest or not. Unit tests will need to be adjusted though. virhostdev.c: check all IOMMU devs in virHostdevPreparePCIDevices formatdomain.html.in: document For the first 4 I'll give it a few days and push on Friday if no one complains. For the last two: utils: PCI multifunction detection helpers domain_conf.c: don't allow function zero to be unassigned The domain_conf.c additions should go into the virDomainHostdevDefValidate. But this validation check seems to actually read from host PCI space. I'm not sure if that's a good idea to do for every XML parse? What's the failure scenario without this error message? Does it fail in an obvious way? If so, maybe it's better to side step the issue and just let it fail if it's a valid configuration. This is a strange scenario TBH. I discussed it a bit with Alex Williamson in the v2 of this series. At first there is nothing wrong with this, in fact QEMU allows it. Problem is that Power guests handle this case in a "better" way than x86 and others, making the non-zero function being visible and usable by the guest without any extra kernel option (x86 needs pci_scan_all). It's also hardware dependent - the BCM5719 network card I am using for testing deals with this scenario, but there's no guarantee that other cards will. To answer your question directly: this might not fail in an obvious way in the guest, but it's not like this feature is a default PCI assignment mode - the user have to deliberately unassigned the function 0 in the XML. Granted, I am being conservative with this extra check - we can simply let the user play with the configuration and, in case it blows up, the user can simply add function 0 back to the guest. If this turns out to be a "toxic" setup then I can go to QEMU and deal with it there - I mean, in the end it's QEMU that allows this, so makes sense to handle the case down there. Thanks, DHB Maybe laine knows better if there's precedent for similar checks at Validate time Thanks, Cole -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH] virt-host-validate: warn if kvm_hv is not loaded for POWER hosts
POWER hosts does not implement CPU virtualization extensions like
x86 or s390x. Instead, all bare-metal POWER hosts are considered
to be virtualization ready.
For POWER, the validation is done by checking the virtualization
kernel modules, kvm_hv and kvm_pr, to see if they are either not
installed or not loaded in the host. If the KVM modules aren't
present, we should not just warn but fail to validate.
This patch implements this support. If kvm_hv is not installed,
which can be determined by 'modinfo' returning not-zero return
code, fail the verification. If kvm_hv is installed but not
loaded, show a warning. The exception are POWER8 hosts, which can
work with kvm_pr. In its case, ACK the use of kvm_pr if kvm_hv
is not loaded/present.
Signed-off-by: Daniel Henrique Barboza
---
tools/virt-host-validate-common.c | 136 ++
tools/virt-host-validate-common.h | 2 +
tools/virt-host-validate-qemu.c | 6 ++
3 files changed, 144 insertions(+)
diff --git a/tools/virt-host-validate-common.c
b/tools/virt-host-validate-common.c
index bce0f14917..e6d7986758 100644
--- a/tools/virt-host-validate-common.c
+++ b/tools/virt-host-validate-common.c
@@ -411,3 +411,139 @@ int virHostValidateIOMMU(const char *hvname,
virHostMsgPass();
return 0;
}
+
+
+static bool virHostCPUIsPower8(void)
+{
+ FILE *fp;
+ bool ret = false;
+
+if (!(fp = fopen("/proc/cpuinfo", "r")))
+return false;
+
+do {
+char line[1024];
+
+if (!fgets(line, sizeof(line), fp))
+break;
+
+/* Looks for the 'model name' line. This is more common for
+ * Intel /proc/cpuinfo formats, but let's account for it
+ * too. */
+if (STRPREFIX(line, "model name")) {
+if (strstr(line, "POWER8"))
+ret = true;
+break;
+}
+
+/* Looks for the 'cpu:' line which is more commonly present
+ * in /proc/cpuinfo Power systems. To ensure this is not
+ * 'cpu id' or any other cpu attribute, peek at the next char
+ * after the first whitespace. A tab, whitespace or ':'
+ * indicates we're on the right line */
+if (STRPREFIX(line, "cpu") &&
+(line[3] == '\t' || line[3] == ':' || line[3] == ' ')) {
+ if (strstr(line, "POWER8"))
+ret = true;
+break;
+}
+
+} while (1);
+
+VIR_FORCE_FCLOSE(fp);
+
+return ret;
+}
+
+
+static bool virHostKernelModuleExists(const char *module)
+{
+g_autofree char *cmd = g_strdup_printf("modinfo %s", module);
+g_autofree char *stdout = NULL;
+g_autofree char *stderr = NULL;
+g_autoptr(GError) err = NULL;
+int errStatus;
+
+if (g_spawn_command_line_sync(cmd, &stdout, &stderr, &errStatus, &err))
+return true;
+
+return false;
+}
+
+
+static bool virHostKernelModuleIsLoaded(const char *module)
+{
+FILE *fp;
+bool ret = false;
+
+if (!(fp = fopen("/proc/modules", "r")))
+return false;
+
+do {
+char line[1024];
+
+if (!fgets(line, sizeof(line), fp))
+break;
+
+if (STRPREFIX(line, module)) {
+ret = true;
+break;
+}
+
+} while (1);
+
+VIR_FORCE_FCLOSE(fp);
+
+return ret;
+}
+
+
+int virHostValidatePowerPCModules(void)
+{
+bool kvm_pr_exists = virHostKernelModuleExists("kvm_pr");
+bool kvm_pr_loaded = kvm_pr_exists &&
virHostKernelModuleIsLoaded("kvm_pr");
+bool kvm_hv_exists = virHostKernelModuleExists("kvm_hv");
+bool kvm_hv_loaded = kvm_hv_exists &&
virHostKernelModuleIsLoaded("kvm_hv");
+bool hostIsP8 = virHostCPUIsPower8();
+
+virHostMsgCheck("QEMU", "%s", _("for PowerPC KVM modules loaded"));
+
+/* No Power KVM virtualization modules present on the host. */
+if (!kvm_hv_exists && !kvm_pr_exists) {
+virHostMsgFail(VIR_HOST_VALIDATE_FAIL,
+ _("No kvm_hv or kvm_pr module present in "
+ "the host"));
+return -1;
+}
+
+/* Bail out for all non-Power8 CPUs if kvm_hv is not present. */
+if (!kvm_hv_exists && !hostIsP8) {
+virHostMsgFail(VIR_HOST_VALIDATE_FAIL,
+ _("No kvm_hv module present in the host"));
+return -1;
+}
+
+/* Power8 CPUs virtualization works with any of kvm_hv and kvm_pr.
+ * Issue a warning if none are loaded. */
+if (hostIsP8) {
+if (!kvm_hv_loaded && !kvm_pr_loaded) {
+virHostMsgFail(VIR_HOST_VALIDATE_WARN,
+ _("Load kvm_hv or kvm_pr module "
+ "for better performance"));
+return 0;
+}
+
+virHostMsgPass();
+return 0;
+}
+
+/* For non-Power8 hosts, show a warning if kvm_hv is not loaded. */
+if (!kvm_hv_loaded) {
+virHostMsgFail(VIR_HOST_VALIDATE_WARN,
+ _("Load kvm_hv for better performance"));
+retur
Re: [libvirt] [PATCH 19/19] tests: qemublock: Add tests for cross-snapshot incremental backups
On 12/12/19 11:18 AM, Peter Krempa wrote: Signed-off-by: Peter Krempa --- tests/qemublocktest.c | 4 ++ .../backupmerge/snapshot-deep-out.json| 38 +++ .../backupmerge/snapshot-flat-out.json| 6 +++ .../snapshot-intermediate-out.json| 14 +++ 4 files changed, 62 insertions(+) create mode 100644 tests/qemublocktestdata/backupmerge/snapshot-deep-out.json create mode 100644 tests/qemublocktestdata/backupmerge/snapshot-flat-out.json create mode 100644 tests/qemublocktestdata/backupmerge/snapshot-intermediate-out.json Reviewed-by: Eric Blake -- Eric Blake, Principal Software Engineer Red Hat, Inc. +1-919-301-3226 Virtualization: qemu.org | libvirt.org -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 18/19] qemu: backup: Merge bitmaps accross the backing chain
On 12/12/19 11:18 AM, Peter Krempa wrote: To allow backups work accross external snapshots we need to improve the across algorithm which calculates which bitmaps to merge. The algorithm must look for appropriately named bitmaps in the image and possibly descend into a backing image if the current image does not have the bitmap. Signed-off-by: Peter Krempa --- src/qemu/qemu_backup.c | 62 -- 1 file changed, 54 insertions(+), 8 deletions(-) Reviewed-by: Eric Blake -- Eric Blake, Principal Software Engineer Red Hat, Inc. +1-919-301-3226 Virtualization: qemu.org | libvirt.org -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 17/19] qemu: block: Introduce qemuBlockNamedNodeDataGetBitmapByName
On 12/12/19 11:18 AM, Peter Krempa wrote: This function looks up a named bitmap for a virStorageSource in the data returned from query-named-block-nodes. Signed-off-by: Peter Krempa --- src/qemu/qemu_block.c | 32 src/qemu/qemu_block.h | 5 + 2 files changed, 37 insertions(+) Reviewed-by: Eric Blake -- Eric Blake, Principal Software Engineer Red Hat, Inc. +1-919-301-3226 Virtualization: qemu.org | libvirt.org -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 16/19] tests: qemublock: Add testing of bitmap merging for incremental backups
On 12/12/19 11:18 AM, Peter Krempa wrote: Add test code which will crawl a fake internal list of checkpoints and generate the list of bitmaps for merging to gather the final bitmap for the backup. The initial tests cover the basic case of all bitmaps being present in the top layer of the backing chain. Signed-off-by: Peter Krempa --- + +static const char *backupDataPrefix = "qemublocktestdata/backupmerge/"; + Drop the double space. Reviewed-by: Eric Blake -- Eric Blake, Principal Software Engineer Red Hat, Inc. +1-919-301-3226 Virtualization: qemu.org | libvirt.org -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 15/19] qemu: backup: Export qemuBackupDiskPrepareOneBitmapsChain for tests
On 12/12/19 11:18 AM, Peter Krempa wrote: Signed-off-by: Peter Krempa --- src/qemu/qemu_backup.c | 2 +- src/qemu/qemu_backup.h | 7 +++ 2 files changed, 8 insertions(+), 1 deletion(-) Reviewed-by: Eric Blake -- Eric Blake, Principal Software Engineer Red Hat, Inc. +1-919-301-3226 Virtualization: qemu.org | libvirt.org -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 14/19] qemu: backup: Propagate bitmap metadata into qemuBackupDiskPrepareOneBitmapsChain
On 12/12/19 11:18 AM, Peter Krempa wrote: The function will require the bitmap topology for the full implementation. To facilitate testing, add the propagation of the necessary data beforehand so that the test code can stay unchanged during the changes.t Stray t. Signed-off-by: Peter Krempa --- src/qemu/qemu_backup.c | 35 ++- 1 file changed, 22 insertions(+), 13 deletions(-) Reviewed-by: Eric Blake -- Eric Blake, Principal Software Engineer Red Hat, Inc. +1-919-301-3226 Virtualization: qemu.org | libvirt.org -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 13/19] qemu: backup: Extract calculations of bitmaps to merge for incremental backup
On 12/12/19 11:18 AM, Peter Krempa wrote:
Separate the for now incomplete code that collects the bitmaps to be
merged for an incremental backup into a separate function. This will
allow to add testing prior to the improvement of the algorithm to
"allow to $VERB" is not idiomatic English; you generally want "allow
$SUBJECT to $VERB" or "allow ${VERB}ing". Here, "This will allow the
addition of testing prior to ..."
include snapshots.
Signed-off-by: Peter Krempa
---
src/qemu/qemu_backup.c | 37 +
1 file changed, 25 insertions(+), 12 deletions(-)
The refactoring is sane.
Reviewed-by: Eric Blake
--
Eric Blake, Principal Software Engineer
Red Hat, Inc. +1-919-301-3226
Virtualization: qemu.org | libvirt.org
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 12/19] qemu: backup: Return 'def' instead of 'obj' from qemuBackupBeginCollectIncrementalCheckpoints
On 12/12/19 11:18 AM, Peter Krempa wrote: The object itself has no extra value and it would make testing the code harder. Refactor it to remove just the definition pointer. Signed-off-by: Peter Krempa --- src/qemu/qemu_backup.c | 29 - 1 file changed, 16 insertions(+), 13 deletions(-) Reviewed-by: Eric Blake -- Eric Blake, Principal Software Engineer Red Hat, Inc. +1-919-301-3226 Virtualization: qemu.org | libvirt.org -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 11/19] tests: qemublock: Add test case for detecting bitmaps as we create snapshots
On 12/12/19 11:18 AM, Peter Krempa wrote:
Add test data gathered from a run of qemu after creating bitmaps and
snapshots together in various combinations.
Signed-off-by: Peter Krempa
---
tests/qemublocktest.c | 1 +
tests/qemublocktestdata/bitmap/snapshots.json | 836 ++
Huge!
tests/qemublocktestdata/bitmap/snapshots.out | 14 +
But looks like a good summary of potential configurations.
3 files changed, 851 insertions(+)
create mode 100644 tests/qemublocktestdata/bitmap/snapshots.json
create mode 100644 tests/qemublocktestdata/bitmap/snapshots.out
+"filename": "/tmp/pull4.1575911550",
+"cluster-size": 65536,
+"format": "qcow2",
+"actual-size": 212992,
+"format-specific": {
+"type": "qcow2",
+"data": {
+"compat": "1.1",
+"lazy-refcounts": false,
+"bitmaps": [
+{
It's annoying that qemu semi-duplicates information between "bitmaps"...
+"backing_file": "/tmp/pull4.1575911540",
+"dirty-bitmaps": [
+{
and "dirty-bitmaps", but you appear to be grabbing the intended field.
+++ b/tests/qemublocktestdata/bitmap/snapshots.out
@@ -0,0 +1,14 @@
+libvirt-1-format:
+d: recod:0 busy:0 persist:1 inconist:0 gran:65536 dirty:0
+ current: recod:1 busy:0 persist:1 inconist:0 gran:65536 dirty:0
More fallout from typo fixes earlier in the series.
Reviewed-by: Eric Blake
+libvirt-2-format:
+c: recod:0 busy:0 persist:1 inconist:0 gran:65536 dirty:0
+d: recod:1 busy:0 persist:1 inconist:0 gran:65536 dirty:0
+libvirt-3-format:
+a: recod:0 busy:0 persist:1 inconist:0 gran:65536 dirty:0
+b: recod:0 busy:0 persist:1 inconist:0 gran:65536 dirty:0
+c: recod:1 busy:0 persist:1 inconist:0 gran:65536 dirty:0
+libvirt-4-format:
+a: recod:1 busy:0 persist:1 inconist:0 gran:65536 dirty:0
+libvirt-5-format:
+a: recod:1 busy:0 persist:1 inconist:0 gran:65536 dirty:0
--
Eric Blake, Principal Software Engineer
Red Hat, Inc. +1-919-301-3226
Virtualization: qemu.org | libvirt.org
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 10/19] qemu: snapshot: Propagate active bitmaps through external snapshots
On 12/12/19 11:18 AM, Peter Krempa wrote:
Re-create any active persistent bitmap in the snapshot overlay image so
that tracking for a checkpoint is persisted. While this basically
duplicates data in the allocation map it's currently the only possible
way as qemu can't mirror the allocation map into a dirty bitmap if we'd
ever want to do a backup.
Signed-off-by: Peter Krempa
---
src/qemu/qemu_driver.c | 39 +++
1 file changed, 39 insertions(+)
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index d2769dab1a..8ccd6b7c97 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -15223,6 +15223,42 @@
qemuDomainSnapshotDiskCleanup(qemuDomainSnapshotDiskDataPtr data,
}
+/**
+ * qemuDomainSnapshotDiskBitmapsPropagate:
+ *
+ * This function propagates any active persistent bitmap present in the
original
+ * image into the new snapshot. We leave the original bitmap active as in cases
+ * when the overlay is discarded (snapshot revert with abandoning the history)
+ * everything works as expected.
That, and the backing image is now read-only so the active bitmap in
that layer won't be getting any more writes until you merge the active
layer back in via commit.
+ * */
Funny looking comment end.
+static int
+qemuDomainSnapshotDiskBitmapsPropagate(qemuDomainSnapshotDiskDataPtr dd,
+ virJSONValuePtr actions,
+ virHashTablePtr blockNamedNodeData)
+{
+qemuBlockNamedNodeDataPtr entry;
+size_t i;
+
+if (!(entry = virHashLookup(blockNamedNodeData,
dd->disk->src->nodeformat)))
+return 0;
+
+for (i = 0; i < entry->nbitmaps; i++) {
+qemuBlockNamedNodeDataBitmapPtr bitmap = entry->bitmaps[i];
+
+/* we don't care about temporary, inconsistent, or disabled bitmaps */
+if (!bitmap->persistent || !bitmap->recording || bitmap->inconsistent)
+continue;
+
+if (qemuMonitorTransactionBitmapAdd(actions, dd->src->nodeformat,
+bitmap->name, true, false,
+bitmap->granularity) < 0)
Ah, you're reproducing whatever granularity already existed (even though
so far, libvirt doesn't expose a knob for setting checkpoint bitmap
granularity, so it will always be the default unless the image was
modified behind libvirt's back).
Reviewed-by: Eric Blake
--
Eric Blake, Principal Software Engineer
Red Hat, Inc. +1-919-301-3226
Virtualization: qemu.org | libvirt.org
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 09/19] qemu: monitor: Add 'granularity' parameter for block-dirty-bitmap-add
On 12/12/19 11:18 AM, Peter Krempa wrote: Signed-off-by: Peter Krempa --- src/qemu/qemu_backup.c | 4 ++-- src/qemu/qemu_checkpoint.c | 2 +- src/qemu/qemu_monitor.c | 6 -- src/qemu/qemu_monitor.h | 3 ++- src/qemu/qemu_monitor_json.c | 4 +++- src/qemu/qemu_monitor_json.h | 3 ++- tests/qemumonitorjsontest.c | 2 +- 7 files changed, 15 insertions(+), 9 deletions(-) Not quite sure where you will use it, but it doesn't hurt to add. Reviewed-by: Eric Blake -- Eric Blake, Principal Software Engineer Red Hat, Inc. +1-919-301-3226 Virtualization: qemu.org | libvirt.org -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 08/19] qemu: snapshot: Fold formatting of snapshot transaction into prepare func
On 12/12/19 11:18 AM, Peter Krempa wrote: qemuDomainSnapshotDiskPrepareOne is already called for each disk which is member of the snapshot so we don't need to iterate through the snapshot list again to generate members of the 'transaction' command for each snapshot. Signed-off-by: Peter Krempa --- src/qemu/qemu_driver.c | 45 +- 1 file changed, 18 insertions(+), 27 deletions(-) Reviewed-by: Eric Blake -- Eric Blake, Principal Software Engineer Red Hat, Inc. +1-919-301-3226 Virtualization: qemu.org | libvirt.org -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 07/19] qemu: Check for explicit failure of qemuBlockSnapshotAddBlockdev
On 12/12/19 11:18 AM, Peter Krempa wrote:
Check that the value is less than 0.
Signed-off-by: Peter Krempa
---
src/qemu/qemu_driver.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index 4d77fd6f6a..41a124d215 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -15533,7 +15533,7 @@ qemuDomainSnapshotCreateDiskActive(virQEMUDriverPtr
driver,
if (blockdev) {
if (qemuBlockSnapshotAddBlockdev(actions,
diskdata[i].disk,
- diskdata[i].src))
+ diskdata[i].src) < 0)
Reviewed-by: Eric Blake
--
Eric Blake, Principal Software Engineer
Red Hat, Inc. +1-919-301-3226
Virtualization: qemu.org | libvirt.org
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 06/19] tests: qemublocktest: Add a syntetic test case for bitmap detection
On 12/12/19 11:18 AM, Peter Krempa wrote: The real data gathered for the 'basic' test case don't excercise some exercise fields. Add a copy with a few values modified. Signed-off-by: Peter Krempa --- +++ b/tests/qemublocktestdata/bitmap/synthetic.out @@ -0,0 +1,6 @@ +libvirt-1-format: + current: recod:1 busy:0 persist:1 inconist:0 gran:65536 dirty:0 +d: recod:0 busy:0 persist:0 inconist:0 gran:65536 dirty:0 +c: recod:0 busy:0 persist:1 inconist:0 gran:1234 dirty:0 +b: recod:0 busy:1 persist:1 inconist:0 gran:65536 dirty:21314 +a: recod:0 busy:0 persist:1 inconist:1 gran:65536 dirty:0 Fallout after typo fixes in 5/19. Reviewed-by: Eric Blake -- Eric Blake, Principal Software Engineer Red Hat, Inc. +1-919-301-3226 Virtualization: qemu.org | libvirt.org -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 05/19] tests: qemublock: Add test for bitmap detection
On 12/12/19 11:18 AM, Peter Krempa wrote:
Test the extraction of data about changed block tracking bitmaps. The
first test case adds a simple scenario of multiple bitmaps in one layer.
The test data will be also later reused for testing the code that
determines which bitmaps to merge for an incremental backup.
Signed-off-by: Peter Krempa
---
tests/qemublocktest.c | 75 ++
+static void
+testQemuDetectBitmapsWorker(virHashTablePtr nodedata,
+const char *nodename,
+virBufferPtr buf)
+{
+qemuBlockNamedNodeDataPtr data;
+size_t i;
+
+if (!(data = virHashLookup(nodedata, nodename)))
+return;
+
+virBufferAsprintf(buf, "%s:\n", nodename);
+virBufferAdjustIndent(buf, 1);
+
+for (i = 0; i < data->nbitmaps; i++) {
+qemuBlockNamedNodeDataBitmapPtr bitmap = data->bitmaps[i];
+
+virBufferAsprintf(buf, "%8s: recod:%d busy:%d persist:%d inconist:%d
gran:%llu dirty:%llu\n",
s/recod/record/
s/inconist/inconsist/
+
+if (!(nodedata = qemuMonitorJSONBlockGetNamedNodeDataJSON(nodedatajson))) {
+VIR_TEST_VERBOSE("failed to load nodedata JSON\n");
Drop the \n (most VIR_TEST_VERBOSE() does not use it)
+++ b/tests/qemublocktestdata/bitmap/basic.json
@@ -0,0 +1,117 @@
+[
+{
+"iops_rd": 0,
+"detect_zeroes": "off",
+"image": {
+"virtual-size": 10485760,
+"filename": "/tmp/pull4.qcow2",
It might be fun to record in the commit message how you created this
temp file. But not strictly necessary.
+++ b/tests/qemublocktestdata/bitmap/basic.out
@@ -0,0 +1,6 @@
+libvirt-1-format:
+ current: recod:1 busy:0 persist:1 inconist:0 gran:65536 dirty:0
+d: recod:0 busy:0 persist:1 inconist:0 gran:65536 dirty:0
+c: recod:0 busy:0 persist:1 inconist:0 gran:65536 dirty:0
+b: recod:0 busy:0 persist:1 inconist:0 gran:65536 dirty:0
+a: recod:0 busy:0 persist:1 inconist:0 gran:65536 dirty:0
Fallout here when you fix the earlier output line.
With typos fixed,
Reviewed-by: Eric Blake
--
Eric Blake, Principal Software Engineer
Red Hat, Inc. +1-919-301-3226
Virtualization: qemu.org | libvirt.org
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 04/19] qemu: monitor: Extract internals of qemuMonitorJSONBlockGetNamedNodeData
On 12/12/19 11:18 AM, Peter Krempa wrote: For testing purposes it will be beneficial to be able to parse the data from JSON directly rather than trying to simulate the monitor. Extract the worker bits and export them. Signed-off-by: Peter Krempa --- src/qemu/qemu_monitor_json.c | 18 +- src/qemu/qemu_monitor_json.h | 3 +++ 2 files changed, 16 insertions(+), 5 deletions(-) Reviewed-by: Eric Blake -- Eric Blake, Principal Software Engineer Red Hat, Inc. +1-919-301-3226 Virtualization: qemu.org | libvirt.org -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [RFC PATCH 0/9] Introduce mediate ops in vfio-pci
On Thu, 12 Dec 2019 12:09:48 +0800 Jason Wang wrote: > On 2019/12/7 上午1:42, Alex Williamson wrote: > > On Fri, 6 Dec 2019 17:40:02 +0800 > > Jason Wang wrote: > > > >> On 2019/12/6 下午4:22, Yan Zhao wrote: > >>> On Thu, Dec 05, 2019 at 09:05:54PM +0800, Jason Wang wrote: > On 2019/12/5 下午4:51, Yan Zhao wrote: > > On Thu, Dec 05, 2019 at 02:33:19PM +0800, Jason Wang wrote: > >> Hi: > >> > >> On 2019/12/5 上午11:24, Yan Zhao wrote: > >>> For SRIOV devices, VFs are passthroughed into guest directly without > >>> host > >>> driver mediation. However, when VMs migrating with passthroughed VFs, > >>> dynamic host mediation is required to (1) get device states, (2) get > >>> dirty pages. Since device states as well as other critical information > >>> required for dirty page tracking for VFs are usually retrieved from > >>> PFs, > >>> it is handy to provide an extension in PF driver to centralizingly > >>> control > >>> VFs' migration. > >>> > >>> Therefore, in order to realize (1) passthrough VFs at normal time, (2) > >>> dynamically trap VFs' bars for dirty page tracking and > >> A silly question, what's the reason for doing this, is this a must for > >> dirty > >> page tracking? > >> > > For performance consideration. VFs' bars should be passthoughed at > > normal time and only enter into trap state on need. > Right, but how does this matter for the case of dirty page tracking? > > >>> Take NIC as an example, to trap its VF dirty pages, software way is > >>> required to trap every write of ring tail that resides in BAR0. > >> > >> Interesting, but it looks like we need: > >> - decode the instruction > >> - mediate all access to BAR0 > >> All of which seems a great burden for the VF driver. I wonder whether or > >> not doing interrupt relay and tracking head is better in this case. > > This sounds like a NIC specific solution, I believe the goal here is to > > allow any device type to implement a partial mediation solution, in > > this case to sufficiently track the device while in the migration > > saving state. > > > I suspect there's a solution that can work for any device type. E.g for > virtio, avail index (head) doesn't belongs to any BAR and device may > decide to disable doorbell from guest. So did interrupt relay since > driver may choose to disable interrupt from device. In this case, the > only way to track dirty pages correctly is to switch to software datapath. > > > > > >>>There's > >>> still no IOMMU Dirty bit available. > >>> (3) centralizing > >>> VF critical states retrieving and VF controls into one driver, we > >>> propose > >>> to introduce mediate ops on top of current vfio-pci device driver. > >>> > >>> > >>>_ _ _ _ _ _ _ _ _ _ _ _ _ _ _ > >>> _ _ > >>> __ register mediate ops| ___ ___ > >>> | > >>> | |<---| VF| | | > >>> | vfio-pci | | | mediate | | PF driver | | > >>> |__|--->| driver | |___| > >>> |open(pdev) | --- | > >>> | > >>> || > >>> ||_ _ _ _ _ _ _ _ _ _ _ _|_ _ _ > >>> _ _| > >>> \|/ \|/ > >>> --- > >>> |VF | |PF| > >>> --- > >>> > >>> > >>> VF mediate driver could be a standalone driver that does not bind to > >>> any devices (as in demo code in patches 5-6) or it could be a built-in > >>> extension of PF driver (as in patches 7-9) . > >>> > >>> Rather than directly bind to VF, VF mediate driver register a mediate > >>> ops into vfio-pci in driver init. vfio-pci maintains a list of such > >>> mediate ops. > >>> (Note that: VF mediate driver can register mediate ops into vfio-pci > >>> before vfio-pci binding to any devices. And VF mediate driver can > >>> support mediating multiple devices.) > >>> > >>> When opening a device (e.g. a VF), vfio-pci goes through the mediate > >>> ops > >>> list and calls each vfio_pci_mediate_ops->open() with pdev of the > >>> opening > >>> device as a parameter. > >>> VF mediate driver should return success or failure depending on it > >>> supports the pdev or not. > >>> E.g. VF mediate driver would compare its supported VF devfn with the > >>> devfn of the passed-in pdev. > >>> Once vfio-pci finds a successful vfio_pci_mediate_ops->
Re: [libvirt] [PATCH 01/19] virsh: Add QMP command wrapping for 'qemu-monitor-command'
On 12/12/19 7:16 PM, Eric Blake wrote:
On 12/12/19 11:18 AM, Peter Krempa wrote:
Issuing simple QMP commands is pain as they need to be wrapped by the
JSON wrapper:
{ "execute": "COMMAND" }
and optionally also:
{ "execute": "COMMAND", "arguments":...}
For simple commands without arguments we can add syntax sugar to virsh
which allows simple usage of QMP and additionally prepares also for
passing through of the 'arguments' section.
I'd give an example of the new syntax in the commit message:
virsh qemu-monitor-command domain --qmp COMMAND '{ARGUMENTS...}'
as shorthand for
virsh qemu-monitor-command domain '"execute":"COMMAND",
"arguments":{ARGUMENTS...}}'
But the sugar is indeed nice (one less layer of {} JSON).
Since we won't use HMP to talk to qemu ever (even the small set of HMP
commands we have are wrapped inside QMP once being sent down the wire),
can we not use --qmp flag at all? Just look if there's "execute" in the
user's input and if not add it there. For instance:
virsh qemu-monitor-command query-machines
will expand to
{"execute":"query-machines"}
and:
virsh qemu-monitor-command '{"execute":"query-machines"}'
will do nothing to the user's input.
Michal
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 03/19] qemu: monitor: Extract data about dirty-bimaps in qemuMonitorBlockGetNamedNodeData
On 12/12/19 11:18 AM, Peter Krempa wrote:
We will need to inspect the presence and attributes for dirty bitmaps.
Extract them when processing reply of query-named-block-nodes.
Signed-off-by: Peter Krempa
---
src/qemu/qemu_monitor.h | 15
src/qemu/qemu_monitor_json.c | 74
2 files changed, 89 insertions(+)
+static void
+qemuMonitorJSONBlockGetNamedNodeDataBitmaps(virJSONValuePtr bitmaps,
+qemuBlockNamedNodeDataPtr data)
+{
+size_t nbitmaps = virJSONValueArraySize(bitmaps);
+size_t i;
+
+data->bitmaps = g_new0(qemuBlockNamedNodeDataBitmapPtr, nbitmaps);
+
+for (i = 0; i < nbitmaps; i++) {
+virJSONValuePtr bitmap = virJSONValueArrayGet(bitmaps, i);
+qemuBlockNamedNodeDataBitmapPtr tmp;
+
+if (!bitmap)
+continue;
Can bitmap ever be NULL? (We could assert that it is not, given our
correct usage of the API - except that we aren't using asserts). But
doesn't hurt to leave the check in.
Reviewed-by: Eric Blake
--
Eric Blake, Principal Software Engineer
Red Hat, Inc. +1-919-301-3226
Virtualization: qemu.org | libvirt.org
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 02/19] virsh: Allow extracting 'return' section of QMP command in 'qemu-monitor-command'
On 12/12/19 11:18 AM, Peter Krempa wrote: Simplify gathering the actual return value from a passed-through QMP command when using 'qemu-monitor-command' by adding '--return-value' switch which just extracts the 'return' section and alternatively reports an error if the section is not present. This simplifies gathering of some test data where the full reply would need to be trimmed just for the actual return value. Signed-off-by: Peter Krempa --- docs/manpages/virsh.rst | 5 - tools/virsh-domain.c| 44 - 2 files changed, 39 insertions(+), 10 deletions(-) Nice use-case. Reviewed-by: Eric Blake -- Eric Blake, Principal Software Engineer Red Hat, Inc. +1-919-301-3226 Virtualization: qemu.org | libvirt.org -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 01/19] virsh: Add QMP command wrapping for 'qemu-monitor-command'
On 12/12/19 11:18 AM, Peter Krempa wrote:
Issuing simple QMP commands is pain as they need to be wrapped by the
JSON wrapper:
{ "execute": "COMMAND" }
and optionally also:
{ "execute": "COMMAND", "arguments":...}
For simple commands without arguments we can add syntax sugar to virsh
which allows simple usage of QMP and additionally prepares also for
passing through of the 'arguments' section.
I'd give an example of the new syntax in the commit message:
virsh qemu-monitor-command domain --qmp COMMAND '{ARGUMENTS...}'
as shorthand for
virsh qemu-monitor-command domain '"execute":"COMMAND",
"arguments":{ARGUMENTS...}}'
But the sugar is indeed nice (one less layer of {} JSON).
Signed-off-by: Peter Krempa
---
docs/manpages/virsh.rst | 24 +---
tools/virsh-domain.c| 32 +---
2 files changed, 46 insertions(+), 10 deletions(-)
-virBufferTrim(&buf, " ", -1);
+if ((opt = vshCommandOptArgv(ctl, cmd, opt)))
+command = opt->data;
+if ((opt = vshCommandOptArgv(ctl, cmd, opt)))
+arguments = opt->data;
+
+if (!command || (arguments && vshCommandOptArgv(ctl, cmd, opt))) {
+vshError(ctl, "%s", _("-qmp option requires 1 or 2 arguments"));
+return false;
Should we allow concatenation and/or magic behavior based on whether the
second argument starts with '{'? For example,
virsh qemu-monitor-command --qmp COMMAND key1=1 'key2="str"'
could be shorthand for
virsh qemu-monitor-command '{"execute":"COMMAND", "arguments":{"key1":1,
"key2":"str"}}'
But further sugar can be a separate patch, so this one works as-is:
Reviewed-by: Eric Blake
--
Eric Blake, Principal Software Engineer
Red Hat, Inc. +1-919-301-3226
Virtualization: qemu.org | libvirt.org
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] tools: skip libvirt-guests fast if libvirtd is not active
On 12/3/19 9:56 AM, Christian Ehrhardt wrote:
The most common operation of libvirt-guests is to manage the local
libvirtd. But users might have disabled that and while we are
After=libvirtd for ordering we are not Requiring it..
OTOH adding that or any harder dependency might affect our ordering.
But if people have disabled libvirt they will do a full retry loop
until timeout. Lets check if the local service is active at all and skip
fast if it is not.
Fixes: https://bugs.launchpad.net/ubuntu/+source/libvirt/+bug/1854653
Reported-by: Doug Smythies
Signed-off-by: Christian Ehrhardt
---
tools/libvirt-guests.sh.in | 8
1 file changed, 8 insertions(+)
diff --git a/tools/libvirt-guests.sh.in b/tools/libvirt-guests.sh.in
index 4bc6e866f0..5a9930ee2f 100644
--- a/tools/libvirt-guests.sh.in
+++ b/tools/libvirt-guests.sh.in
@@ -90,6 +90,14 @@ test_connect()
{
uri=$1
+if [ "x$uri" = xdefault ]; then
+# Default config is most common and for the local libvirtd
+# Check if it is active before wasting time in connect loop
+if ! systemctl -q is-active libvirtd; then
Systemd is still not the only init out there:
moe ~ # systemctl -q is-active libvirtd
-su: systemctl: command not found
However, this will make libvirt-guests unusable when using split daemons
(which are not enabled yet). So I guess we need to check whether either
libvirtd or any of the hypervisor daemons is running (we can't assume
the default URI is qemu:///system). Also, it would be nice to print a
message why we tested connection unusable. Something like:
eval_gettext "Libvirtd is not running. Skipping."
return 1
Michal
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [jenkins-ci PATCH] lcitool: Don't install yast2_base group on openSUSE
On 12/12/19 10:46 AM, Andrea Bolognani wrote: > We're handling all configuration using lcitool, and we have to > be able to cope with minimal openSUSE environments anyway because > of containers. > > Signed-off-by: Andrea Bolognani > --- > I've tested this by creating a fresh openSUSE Leap 15.1 VM and > verifying that I could successfully 'lcitool build' libvirt inside > it. > > guests/configs/autoinst.xml | 1 - > 1 file changed, 1 deletion(-) > > diff --git a/guests/configs/autoinst.xml b/guests/configs/autoinst.xml > index f8ec3df..ceb702d 100644 > --- a/guests/configs/autoinst.xml > +++ b/guests/configs/autoinst.xml > @@ -44,7 +44,6 @@ > > base > minimal_base > - yast2_basis > > > openssh > I suppose I could have sent this myself faster than typing the mails we've exchanged :-). I didn't test it, but Reviewed-by: Jim Fehlig Regards, Jim -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] Remove redundant usage of WITH_YAJL
On 12/12/19 6:22 PM, Ján Tomko wrote: As of commit 2a00ef6e71f30241f9ca6288da984d75f3cef957 which was released in v5.2.0, we require YAJL to build the QEMU driver. Remove the checks from code that requires the QEMU driver or checks that also check for WITH_QEMU. Signed-off-by: Ján Tomko --- src/qemu/qemu_driver.c | 5 - tests/cputest.c | 16 +++--- tests/qemuagenttest.c | 5 - tests/qemublocktest.c | 42 ++--- tests/qemumigparamstest.c | 5 - tests/qemumonitorjsontest.c | 5 - 6 files changed, 24 insertions(+), 54 deletions(-) You can also skip checks from: tests/qemucapabilitiestest.c tests/qemucaps2xmltest.c tests/qemucommandutiltest.c tests/qemuhotplugtest.c since these are ale guarded by WITH_QEMU check which can be true only if WITH_YAJL is true. With that: Reviewed-by: Michal Privoznik Michal -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [jenkins-ci PATCH] lcitool: Don't install yast2_base group on openSUSE
We're handling all configuration using lcitool, and we have to be able to cope with minimal openSUSE environments anyway because of containers. Signed-off-by: Andrea Bolognani --- I've tested this by creating a fresh openSUSE Leap 15.1 VM and verifying that I could successfully 'lcitool build' libvirt inside it. guests/configs/autoinst.xml | 1 - 1 file changed, 1 deletion(-) diff --git a/guests/configs/autoinst.xml b/guests/configs/autoinst.xml index f8ec3df..ceb702d 100644 --- a/guests/configs/autoinst.xml +++ b/guests/configs/autoinst.xml @@ -44,7 +44,6 @@ base minimal_base - yast2_basis openssh -- 2.23.0 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH] Remove redundant usage of WITH_YAJL
As of commit 2a00ef6e71f30241f9ca6288da984d75f3cef957 which
was released in v5.2.0, we require YAJL to build the QEMU driver.
Remove the checks from code that requires the QEMU driver
or checks that also check for WITH_QEMU.
Signed-off-by: Ján Tomko
---
src/qemu/qemu_driver.c | 5 -
tests/cputest.c | 16 +++---
tests/qemuagenttest.c | 5 -
tests/qemublocktest.c | 42 ++---
tests/qemumigparamstest.c | 5 -
tests/qemumonitorjsontest.c | 5 -
6 files changed, 24 insertions(+), 54 deletions(-)
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index e65150f42b..66c58d190f 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -2104,11 +2104,6 @@ qemuDomainRebootMonitor(virQEMUDriverPtr driver,
if (virDomainObjCheckActive(vm) < 0)
goto endjob;
-#if !WITH_YAJL
-virReportError(VIR_ERR_OPERATION_INVALID, "%s",
- _("ACPI reboot is not supported without the JSON monitor"));
-goto endjob;
-#endif
qemuDomainSetFakeReboot(driver, vm, isReboot);
qemuDomainObjEnterMonitor(driver, vm);
ret = qemuMonitorSystemPowerdown(priv->mon);
diff --git a/tests/cputest.c b/tests/cputest.c
index fd86344ea4..f57516838b 100644
--- a/tests/cputest.c
+++ b/tests/cputest.c
@@ -36,7 +36,7 @@
#include "cpu/cpu_map.h"
#include "virstring.h"
-#if WITH_QEMU && WITH_YAJL
+#if WITH_QEMU
# include "testutilsqemu.h"
# include "qemumonitortestutils.h"
# define LIBVIRT_QEMU_CAPSPRIV_H_ALLOW
@@ -62,7 +62,7 @@ struct data {
int result;
};
-#if WITH_QEMU && WITH_YAJL
+#if WITH_QEMU
static virQEMUDriver driver;
#endif
@@ -465,7 +465,7 @@ typedef enum {
JSON_MODELS_REQUIRED,
} cpuTestCPUIDJson;
-#if WITH_QEMU && WITH_YAJL
+#if WITH_QEMU
static virQEMUCapsPtr
cpuTestMakeQEMUCaps(const struct data *data)
{
@@ -547,7 +547,7 @@ cpuTestGetCPUModels(const struct data *data,
return 0;
}
-#else /* if WITH_QEMU && WITH_YAJL */
+#else /* if WITH_QEMU */
static int
cpuTestGetCPUModels(const struct data *data,
@@ -871,7 +871,7 @@ cpuTestUpdateLive(const void *arg)
}
-#if WITH_QEMU && WITH_YAJL
+#if WITH_QEMU
static int
cpuTestJSONCPUID(const void *arg)
{
@@ -970,7 +970,7 @@ mymain(void)
virDomainCapsCPUModelsPtr ppc_models = NULL;
int ret = 0;
-#if WITH_QEMU && WITH_YAJL
+#if WITH_QEMU
if (qemuTestDriverInit(&driver) < 0)
return EXIT_FAILURE;
@@ -1057,7 +1057,7 @@ mymain(void)
host "/" cpu " (" #models ")", \
host, cpu, models, 0, result)
-#if WITH_QEMU && WITH_YAJL
+#if WITH_QEMU
# define DO_TEST_JSON(arch, host, json) \
do { \
if (json == JSON_MODELS) { \
@@ -1270,7 +1270,7 @@ mymain(void)
DO_TEST_CPUID(VIR_ARCH_X86_64, "Ice-Lake-Server", JSON_MODELS);
cleanup:
-#if WITH_QEMU && WITH_YAJL
+#if WITH_QEMU
qemuTestDriverFree(&driver);
#endif
diff --git a/tests/qemuagenttest.c b/tests/qemuagenttest.c
index d8824b8f09..644dc9d08b 100644
--- a/tests/qemuagenttest.c
+++ b/tests/qemuagenttest.c
@@ -1426,11 +1426,6 @@ mymain(void)
{
int ret = 0;
-#if !WITH_YAJL
-fputs("libvirt not compiled with JSON support, skipping this test\n",
stderr);
-return EXIT_AM_SKIP;
-#endif
-
if (qemuTestDriverInit(&driver) < 0)
return EXIT_FAILURE;
diff --git a/tests/qemublocktest.c b/tests/qemublocktest.c
index 2c170548ec..c9335774b9 100644
--- a/tests/qemublocktest.c
+++ b/tests/qemublocktest.c
@@ -18,22 +18,20 @@
#include "testutils.h"
+#include "testutilsqemu.h"
+#include "testutilsqemuschema.h"
+#include "virstoragefile.h"
+#include "virstring.h"
+#include "virlog.h"
+#include "qemu/qemu_block.h"
+#include "qemu/qemu_qapi.h"
-#if WITH_YAJL
-# include "testutilsqemu.h"
-# include "testutilsqemuschema.h"
-# include "virstoragefile.h"
-# include "virstring.h"
-# include "virlog.h"
-# include "qemu/qemu_block.h"
-# include "qemu/qemu_qapi.h"
+#include "qemu/qemu_command.h"
-# include "qemu/qemu_command.h"
+#define LIBVIRT_SNAPSHOT_CONF_PRIV_H_ALLOW
+#include "conf/snapshot_conf_priv.h"
-# define LIBVIRT_SNAPSHOT_CONF_PRIV_H_ALLOW
-# include "conf/snapshot_conf_priv.h"
-
-# define VIR_FROM_THIS VIR_FROM_NONE
+#define VIR_FROM_THIS VIR_FROM_NONE
VIR_LOG_INIT("tests.storagetest");
@@ -523,7 +521,7 @@ mymain(void)
virTestCounterReset("qemu storage source xml->json->xml ");
-# define TEST_JSON_FORMAT(tpe, xmlstr) \
+#define TEST_JSON_FORMAT(tpe, xmlstr) \
do { \
xmljsonxmldata.type = tpe; \
xmljsonxmldata.xml = xmlstr; \
@@ -532,7 +530,7 @@ mymain(void)
ret = -1; \
} while (0)
-# define TEST_JSON_FORMAT_NET(xmlstr) \
+#define TEST_JSON_FORMAT_NET(xmlstr) \
TEST_JSON_FORMAT(VIR_STORAGE_TYPE_NETWORK, xmlstr)
TEST_JSON_FORMAT(VIR_STORAGE_TYPE_FILE, "\n");
@@ -588,7 +586,7 @@ mymain(void)
" \n"
"\n");
-# define TEST_DISK_TO_JSON_F
[libvirt] [PATCH 18/19] qemu: backup: Merge bitmaps accross the backing chain
To allow backups work accross external snapshots we need to improve the
algorithm which calculates which bitmaps to merge.
The algorithm must look for appropriately named bitmaps in the image and
possibly descend into a backing image if the current image does not have
the bitmap.
Signed-off-by: Peter Krempa
---
src/qemu/qemu_backup.c | 62 --
1 file changed, 54 insertions(+), 8 deletions(-)
diff --git a/src/qemu/qemu_backup.c b/src/qemu/qemu_backup.c
index 14cf6bbef0..294d5999a0 100644
--- a/src/qemu/qemu_backup.c
+++ b/src/qemu/qemu_backup.c
@@ -173,24 +173,70 @@ qemuBackupDiskDataCleanup(virDomainObjPtr vm,
virJSONValuePtr
qemuBackupDiskPrepareOneBitmapsChain(virDomainMomentDefPtr *incremental,
virStorageSourcePtr backingChain,
- virHashTablePtr blockNamedNodeData
G_GNUC_UNUSED,
- const char *diskdst G_GNUC_UNUSED)
+ virHashTablePtr blockNamedNodeData,
+ const char *diskdst)
{
+qemuBlockNamedNodeDataBitmapPtr bitmap;
g_autoptr(virJSONValue) ret = NULL;
+size_t incridx = 0;
if (!(ret = virJSONValueNewArray()))
return NULL;
-/* TODO: this code works only if the bitmaps are present on a single node.
- * The algorithm needs to be changed so that it looks into the backing
chain
- * so that we can combine all relevant bitmaps for a given backing chain */
-while (*incremental) {
+if (!(bitmap = qemuBlockNamedNodeDataGetBitmapByName(blockNamedNodeData,
+ backingChain,
+
incremental[0]->name))) {
+virReportError(VIR_ERR_INVALID_ARG,
+ _("failed to find bitmap '%s' in image '%s%u'"),
+ incremental[0]->name, diskdst, backingChain->id);
+return NULL;
+}
+
+while (bitmap) {
+if (bitmap->inconsistent) {
+virReportError(VIR_ERR_INVALID_ARG,
+ _("bitmap '%s' for image '%s%u' is inconsistent"),
+ bitmap->name, diskdst, backingChain->id);
+return NULL;
+}
+
if (qemuMonitorTransactionBitmapMergeSourceAddBitmap(ret,
backingChain->nodeformat,
-
(*incremental)->name) < 0)
+ bitmap->name) < 0)
return NULL;
-incremental++;
+if (backingChain->backingStore &&
+(bitmap = qemuBlockNamedNodeDataGetBitmapByName(blockNamedNodeData,
+
backingChain->backingStore,
+
incremental[incridx]->name))) {
+backingChain = backingChain->backingStore;
+continue;
+}
+
+if (incremental[incridx + 1]) {
+if ((bitmap =
qemuBlockNamedNodeDataGetBitmapByName(blockNamedNodeData,
+backingChain,
+
incremental[incridx + 1]->name))) {
+incridx++;
+continue;
+}
+
+if (backingChain->backingStore &&
+(bitmap =
qemuBlockNamedNodeDataGetBitmapByName(blockNamedNodeData,
+
backingChain->backingStore,
+
incremental[incridx + 1]->name))) {
+incridx++;
+backingChain = backingChain->backingStore;
+continue;
+}
+
+virReportError(VIR_ERR_INVALID_ARG,
+ _("failed to find bitmap '%s' in image '%s%u'"),
+ incremental[incridx]->name, diskdst,
backingChain->id);
+return NULL;
+} else {
+break;
+}
}
return g_steal_pointer(&ret);
--
2.23.0
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 07/19] qemu: Check for explicit failure of qemuBlockSnapshotAddBlockdev
Check that the value is less than 0.
Signed-off-by: Peter Krempa
---
src/qemu/qemu_driver.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index 4d77fd6f6a..41a124d215 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -15533,7 +15533,7 @@ qemuDomainSnapshotCreateDiskActive(virQEMUDriverPtr
driver,
if (blockdev) {
if (qemuBlockSnapshotAddBlockdev(actions,
diskdata[i].disk,
- diskdata[i].src))
+ diskdata[i].src) < 0)
goto cleanup;
} else {
if (qemuBlockSnapshotAddLegacy(actions,
--
2.23.0
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 19/19] tests: qemublock: Add tests for cross-snapshot incremental backups
Signed-off-by: Peter Krempa
---
tests/qemublocktest.c | 4 ++
.../backupmerge/snapshot-deep-out.json| 38 +++
.../backupmerge/snapshot-flat-out.json| 6 +++
.../snapshot-intermediate-out.json| 14 +++
4 files changed, 62 insertions(+)
create mode 100644 tests/qemublocktestdata/backupmerge/snapshot-deep-out.json
create mode 100644 tests/qemublocktestdata/backupmerge/snapshot-flat-out.json
create mode 100644
tests/qemublocktestdata/backupmerge/snapshot-intermediate-out.json
diff --git a/tests/qemublocktest.c b/tests/qemublocktest.c
index 659ce327dc..905a5b6ed2 100644
--- a/tests/qemublocktest.c
+++ b/tests/qemublocktest.c
@@ -939,6 +939,10 @@ mymain(void)
TEST_BACKUP_BITMAP_CALCULATE("basic-intermediate", bitmapSourceChain, "d",
"basic");
TEST_BACKUP_BITMAP_CALCULATE("basic-deep", bitmapSourceChain, "a",
"basic");
+TEST_BACKUP_BITMAP_CALCULATE("snapshot-flat", bitmapSourceChain,
"current", "snapshots");
+TEST_BACKUP_BITMAP_CALCULATE("snapshot-intermediate", bitmapSourceChain,
"d", "snapshots");
+TEST_BACKUP_BITMAP_CALCULATE("snapshot-deep", bitmapSourceChain, "a",
"snapshots");
+
cleanup:
virHashFree(diskxmljsondata.schema);
qemuTestDriverFree(&driver);
diff --git a/tests/qemublocktestdata/backupmerge/snapshot-deep-out.json
b/tests/qemublocktestdata/backupmerge/snapshot-deep-out.json
new file mode 100644
index 00..526fc8d55b
--- /dev/null
+++ b/tests/qemublocktestdata/backupmerge/snapshot-deep-out.json
@@ -0,0 +1,38 @@
+[
+ {
+"node": "libvirt-1-format",
+"name": "current"
+ },
+ {
+"node": "libvirt-1-format",
+"name": "d"
+ },
+ {
+"node": "libvirt-2-format",
+"name": "d"
+ },
+ {
+"node": "libvirt-2-format",
+"name": "c"
+ },
+ {
+"node": "libvirt-3-format",
+"name": "c"
+ },
+ {
+"node": "libvirt-3-format",
+"name": "b"
+ },
+ {
+"node": "libvirt-3-format",
+"name": "a"
+ },
+ {
+"node": "libvirt-4-format",
+"name": "a"
+ },
+ {
+"node": "libvirt-5-format",
+"name": "a"
+ }
+]
diff --git a/tests/qemublocktestdata/backupmerge/snapshot-flat-out.json
b/tests/qemublocktestdata/backupmerge/snapshot-flat-out.json
new file mode 100644
index 00..b89252e284
--- /dev/null
+++ b/tests/qemublocktestdata/backupmerge/snapshot-flat-out.json
@@ -0,0 +1,6 @@
+[
+ {
+"node": "libvirt-1-format",
+"name": "current"
+ }
+]
diff --git a/tests/qemublocktestdata/backupmerge/snapshot-intermediate-out.json
b/tests/qemublocktestdata/backupmerge/snapshot-intermediate-out.json
new file mode 100644
index 00..537d776ec6
--- /dev/null
+++ b/tests/qemublocktestdata/backupmerge/snapshot-intermediate-out.json
@@ -0,0 +1,14 @@
+[
+ {
+"node": "libvirt-1-format",
+"name": "current"
+ },
+ {
+"node": "libvirt-1-format",
+"name": "d"
+ },
+ {
+"node": "libvirt-2-format",
+"name": "d"
+ }
+]
--
2.23.0
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 14/19] qemu: backup: Propagate bitmap metadata into qemuBackupDiskPrepareOneBitmapsChain
The function will require the bitmap topology for the full
implementation. To facilitate testing, add the propagation of the
necessary data beforehand so that the test code can stay unchanged
during the changes.t
Signed-off-by: Peter Krempa
---
src/qemu/qemu_backup.c | 35 ++-
1 file changed, 22 insertions(+), 13 deletions(-)
diff --git a/src/qemu/qemu_backup.c b/src/qemu/qemu_backup.c
index 31949b5399..fac6592366 100644
--- a/src/qemu/qemu_backup.c
+++ b/src/qemu/qemu_backup.c
@@ -172,7 +172,9 @@ qemuBackupDiskDataCleanup(virDomainObjPtr vm,
static virJSONValuePtr
qemuBackupDiskPrepareOneBitmapsChain(virDomainMomentDefPtr *incremental,
- virStorageSourcePtr backingChain)
+ virStorageSourcePtr backingChain,
+ virHashTablePtr blockNamedNodeData
G_GNUC_UNUSED,
+ const char *diskdst G_GNUC_UNUSED)
{
g_autoptr(virJSONValue) ret = NULL;
@@ -198,13 +200,16 @@
qemuBackupDiskPrepareOneBitmapsChain(virDomainMomentDefPtr *incremental,
static int
qemuBackupDiskPrepareOneBitmaps(struct qemuBackupDiskData *dd,
virJSONValuePtr actions,
-virDomainMomentDefPtr *incremental)
+virDomainMomentDefPtr *incremental,
+virHashTablePtr blockNamedNodeData)
{
g_autoptr(virJSONValue) mergebitmaps = NULL;
g_autoptr(virJSONValue) mergebitmapsstore = NULL;
if (!(mergebitmaps = qemuBackupDiskPrepareOneBitmapsChain(incremental,
-
dd->domdisk->src)))
+ dd->domdisk->src,
+
blockNamedNodeData,
+
dd->domdisk->dst)))
return -1;
if (!(mergebitmapsstore = virJSONValueCopy(mergebitmaps)))
@@ -246,6 +251,7 @@ qemuBackupDiskPrepareDataOne(virDomainObjPtr vm,
struct qemuBackupDiskData *dd,
virJSONValuePtr actions,
virDomainMomentDefPtr *incremental,
+ virHashTablePtr blockNamedNodeData,
virQEMUDriverConfigPtr cfg,
bool removeStore)
{
@@ -274,7 +280,8 @@ qemuBackupDiskPrepareDataOne(virDomainObjPtr vm,
if (incremental) {
dd->incrementalBitmap = g_strdup_printf("backup-%s", dd->domdisk->dst);
-if (qemuBackupDiskPrepareOneBitmaps(dd, actions, incremental) < 0)
+if (qemuBackupDiskPrepareOneBitmaps(dd, actions, incremental,
+blockNamedNodeData) < 0)
return -1;
}
@@ -337,6 +344,7 @@ static ssize_t
qemuBackupDiskPrepareData(virDomainObjPtr vm,
virDomainBackupDefPtr def,
virDomainMomentDefPtr *incremental,
+ virHashTablePtr blockNamedNodeData,
virJSONValuePtr actions,
virQEMUDriverConfigPtr cfg,
struct qemuBackupDiskData **rdd,
@@ -359,7 +367,8 @@ qemuBackupDiskPrepareData(virDomainObjPtr vm,
ndisks++;
if (qemuBackupDiskPrepareDataOne(vm, backupdisk, dd, actions,
- incremental, cfg, removeStore) < 0)
+ incremental, blockNamedNodeData,
+ cfg, removeStore) < 0)
goto error;
if (def->type == VIR_DOMAIN_BACKUP_TYPE_PULL) {
@@ -745,8 +754,14 @@ qemuBackupBegin(virDomainObjPtr vm,
goto endjob;
}
-if ((ndd = qemuBackupDiskPrepareData(vm, def, incremental, actions, cfg,
&dd,
- reuse)) <= 0) {
+if (qemuDomainObjEnterMonitorAsync(priv->driver, vm,
QEMU_ASYNC_JOB_BACKUP) < 0)
+goto endjob;
+blockNamedNodeData = qemuMonitorBlockGetNamedNodeData(priv->mon);
+if (qemuDomainObjExitMonitor(priv->driver, vm) < 0 || !blockNamedNodeData)
+goto endjob;
+
+if ((ndd = qemuBackupDiskPrepareData(vm, def, incremental,
blockNamedNodeData,
+ actions, cfg, &dd, reuse)) <= 0) {
if (ndd == 0) {
virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
_("no disks selected for backup"));
@@ -755,12 +770,6 @@ qemuBackupBegin(virDomainObjPtr vm,
goto endjob;
}
-if (qemuDomainObjEnterMonitorAsync(priv->driver, vm,
QEMU_ASYNC_JOB_BACKUP) < 0)
-goto endjob;
-blockNamedNodeData = qemuMonitorBlockGetNamedNodeData(priv->mon);
-if (qemuDomainObjExitMonitor(priv->driver, vm) < 0 || !blockNamedNodeData)
-g
[libvirt] [PATCH 16/19] tests: qemublock: Add testing of bitmap merging for incremental backups
Add test code which will crawl a fake internal list of checkpoints and
generate the list of bitmaps for merging to gather the final bitmap for
the backup.
The initial tests cover the basic case of all bitmaps being present in
the top layer of the backing chain.
Signed-off-by: Peter Krempa
---
tests/qemublocktest.c | 160 ++
.../backupmerge/basic-deep-out.json | 22 +++
.../backupmerge/basic-flat-out.json | 6 +
.../backupmerge/basic-intermediate-out.json | 10 ++
4 files changed, 198 insertions(+)
create mode 100644 tests/qemublocktestdata/backupmerge/basic-deep-out.json
create mode 100644 tests/qemublocktestdata/backupmerge/basic-flat-out.json
create mode 100644
tests/qemublocktestdata/backupmerge/basic-intermediate-out.json
diff --git a/tests/qemublocktest.c b/tests/qemublocktest.c
index 25afef46bb..659ce327dc 100644
--- a/tests/qemublocktest.c
+++ b/tests/qemublocktest.c
@@ -28,6 +28,7 @@
# include "qemu/qemu_block.h"
# include "qemu/qemu_qapi.h"
# include "qemu/qemu_monitor_json.h"
+# include "qemu/qemu_backup.h"
# include "qemu/qemu_command.h"
@@ -558,6 +559,145 @@ testQemuDetectBitmaps(const void *opaque)
}
+static virStorageSourcePtr
+testQemuBackupIncrementalBitmapCalculateGetFakeImage(size_t idx)
+{
+ virStorageSourcePtr ret;
+
+ if (!(ret = virStorageSourceNew()))
+ abort();
+
+ ret->type = VIR_STORAGE_TYPE_FILE;
+ ret->format = VIR_STORAGE_FILE_QCOW2;
+ ret->path = g_strdup_printf("/image%zu", idx);
+ ret->nodestorage = g_strdup_printf("libvirt-%zu-storage", idx);
+ ret->nodeformat = g_strdup_printf("libvirt-%zu-format", idx);
+
+ return ret;
+}
+
+
+static virStorageSourcePtr
+testQemuBackupIncrementalBitmapCalculateGetFakeChain(void)
+{
+virStorageSourcePtr ret;
+virStorageSourcePtr n;
+size_t i;
+
+n = ret = testQemuBackupIncrementalBitmapCalculateGetFakeImage(1);
+
+for (i = 2; i < 10; i++) {
+n->backingStore =
testQemuBackupIncrementalBitmapCalculateGetFakeImage(i);
+n = n->backingStore;
+}
+
+return ret;
+}
+
+
+typedef virDomainMomentDefPtr testMomentList;
+
+static void
+testMomentListFree(testMomentList *list)
+{
+testMomentList *tmp = list;
+
+if (!list)
+return;
+
+while (*tmp) {
+virObjectUnref(*tmp);
+tmp++;
+}
+
+g_free(list);
+}
+
+G_DEFINE_AUTOPTR_CLEANUP_FUNC(testMomentList, testMomentListFree);
+
+static virDomainMomentDefPtr
+testQemuBackupGetIncrementalMoment(const char *name)
+{
+virDomainCheckpointDefPtr checkpoint = NULL;
+
+if (!(checkpoint = virDomainCheckpointDefNew()))
+abort();
+
+checkpoint->parent.name = g_strdup(name);
+
+return (virDomainMomentDefPtr) checkpoint;
+}
+
+
+static virDomainMomentDefPtr *
+testQemuBackupGetIncremental(const char *incFrom)
+{
+const char *checkpoints[] = {"current", "d", "c", "b", "a"};
+virDomainMomentDefPtr *incr;
+size_t i;
+
+incr = g_new0(virDomainMomentDefPtr, G_N_ELEMENTS(checkpoints) + 1);
+
+for (i = 0; i < G_N_ELEMENTS(checkpoints); i++) {
+incr[i] = testQemuBackupGetIncrementalMoment(checkpoints[i]);
+
+if (STREQ(incFrom, checkpoints[i]))
+break;
+}
+
+return incr;
+}
+
+static const char *backupDataPrefix = "qemublocktestdata/backupmerge/";
+
+struct testQemuBackupIncrementalBitmapCalculateData {
+const char *name;
+virStorageSourcePtr chain;
+const char *incremental;
+const char *nodedatafile;
+};
+
+
+static int
+testQemuBackupIncrementalBitmapCalculate(const void *opaque)
+{
+const struct testQemuBackupIncrementalBitmapCalculateData *data = opaque;
+g_autoptr(virJSONValue) nodedatajson = NULL;
+g_autoptr(virHashTable) nodedata = NULL;
+g_autoptr(virJSONValue) mergebitmaps = NULL;
+g_autofree char *actual = NULL;
+g_autofree char *expectpath = NULL;
+g_autoptr(testMomentList) incremental = NULL;
+
+expectpath = g_strdup_printf("%s/%s%s-out.json", abs_srcdir,
+ backupDataPrefix, data->name);
+
+if (!(nodedatajson = virTestLoadFileJSON(bitmapDetectPrefix,
data->nodedatafile,
+ ".json", NULL)))
+return -1;
+
+if (!(nodedata = qemuMonitorJSONBlockGetNamedNodeDataJSON(nodedatajson))) {
+VIR_TEST_VERBOSE("failed to load nodedata JSON\n");
+return -1;
+}
+
+incremental = testQemuBackupGetIncremental(data->incremental);
+
+if (!(mergebitmaps = qemuBackupDiskPrepareOneBitmapsChain(incremental,
+ data->chain,
+ nodedata,
+ "testdisk"))) {
+VIR_TEST_VERBOSE("failed to calculate merged bitmaps");
+return -1;
+}
+
+if (!(actual = virJSONValueToString(mergebitmaps, true)))
+return -1;
+
+
[libvirt] [PATCH 15/19] qemu: backup: Export qemuBackupDiskPrepareOneBitmapsChain for tests
Signed-off-by: Peter Krempa --- src/qemu/qemu_backup.c | 2 +- src/qemu/qemu_backup.h | 7 +++ 2 files changed, 8 insertions(+), 1 deletion(-) diff --git a/src/qemu/qemu_backup.c b/src/qemu/qemu_backup.c index fac6592366..14cf6bbef0 100644 --- a/src/qemu/qemu_backup.c +++ b/src/qemu/qemu_backup.c @@ -170,7 +170,7 @@ qemuBackupDiskDataCleanup(virDomainObjPtr vm, } -static virJSONValuePtr +virJSONValuePtr qemuBackupDiskPrepareOneBitmapsChain(virDomainMomentDefPtr *incremental, virStorageSourcePtr backingChain, virHashTablePtr blockNamedNodeData G_GNUC_UNUSED, diff --git a/src/qemu/qemu_backup.h b/src/qemu/qemu_backup.h index 0f76abe067..df67b849be 100644 --- a/src/qemu/qemu_backup.h +++ b/src/qemu/qemu_backup.h @@ -44,3 +44,10 @@ int qemuBackupGetJobInfoStats(virQEMUDriverPtr driver, virDomainObjPtr vm, qemuDomainJobInfoPtr jobInfo); + +/* exported for testing */ +virJSONValuePtr +qemuBackupDiskPrepareOneBitmapsChain(virDomainMomentDefPtr *incremental, + virStorageSourcePtr backingChain, + virHashTablePtr blockNamedNodeData, + const char *diskdst); -- 2.23.0 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 06/19] tests: qemublocktest: Add a syntetic test case for bitmap detection
The real data gathered for the 'basic' test case don't excercise some
fields. Add a copy with a few values modified.
Signed-off-by: Peter Krempa
---
tests/qemublocktest.c | 1 +
tests/qemublocktestdata/bitmap/synthetic.json | 118 ++
tests/qemublocktestdata/bitmap/synthetic.out | 6 +
3 files changed, 125 insertions(+)
create mode 100644 tests/qemublocktestdata/bitmap/synthetic.json
create mode 100644 tests/qemublocktestdata/bitmap/synthetic.out
diff --git a/tests/qemublocktest.c b/tests/qemublocktest.c
index 16bee47a12..e3aee03c81 100644
--- a/tests/qemublocktest.c
+++ b/tests/qemublocktest.c
@@ -776,6 +776,7 @@ mymain(void)
} while (0)
TEST_BITMAP_DETECT("basic");
+TEST_BITMAP_DETECT("synthetic");
cleanup:
virHashFree(diskxmljsondata.schema);
diff --git a/tests/qemublocktestdata/bitmap/synthetic.json
b/tests/qemublocktestdata/bitmap/synthetic.json
new file mode 100644
index 00..56882bd615
--- /dev/null
+++ b/tests/qemublocktestdata/bitmap/synthetic.json
@@ -0,0 +1,118 @@
+[
+{
+"iops_rd": 0,
+"detect_zeroes": "off",
+"image": {
+"virtual-size": 10485760,
+"filename": "/tmp/pull4.qcow2",
+"cluster-size": 65536,
+"format": "qcow2",
+"actual-size": 200704,
+"format-specific": {
+"type": "qcow2",
+"data": {
+"compat": "1.1",
+"lazy-refcounts": false,
+"refcount-bits": 16,
+"corrupt": false
+}
+},
+"dirty-flag": false
+},
+"iops_wr": 0,
+"ro": false,
+"node-name": "libvirt-1-format",
+"backing_file_depth": 0,
+"drv": "qcow2",
+"iops": 0,
+"bps_wr": 0,
+"write_threshold": 0,
+"dirty-bitmaps": [
+{
+"name": "current",
+"recording": true,
+"persistent": true,
+"busy": false,
+"status": "active",
+"granularity": 65536,
+"count": 0
+},
+{
+"name": "d",
+"recording": false,
+"persistent": false,
+"busy": false,
+"status": "disabled",
+"granularity": 65536,
+"count": 0
+},
+{
+"name": "c",
+"recording": false,
+"persistent": true,
+"busy": false,
+"status": "disabled",
+"granularity": 1234,
+"count": 0
+},
+{
+"name": "b",
+"recording": false,
+"persistent": true,
+"busy": true,
+"status": "disabled",
+"granularity": 65536,
+"count": 21314
+},
+{
+"name": "a",
+"recording": false,
+"persistent": true,
+"busy": false,
+"status": "disabled",
+"inconsistent": true,
+"granularity": 65536,
+"count": 0
+}
+],
+"encrypted": false,
+"bps": 0,
+"bps_rd": 0,
+"cache": {
+"no-flush": false,
+"direct": false,
+"writeback": true
+},
+"file": "/tmp/pull4.qcow2",
+"encryption_key_missing": false
+},
+{
+"iops_rd": 0,
+"detect_zeroes": "off",
+"image": {
+"virtual-size": 197120,
+"filename": "/tmp/pull4.qcow2",
+"format": "file",
+"actual-size": 200704,
+"dirty-flag": false
+},
+"iops_wr": 0,
+"ro": false,
+"node-name": "libvirt-1-storage",
+"backing_file_depth": 0,
+"drv": "file",
+"iops": 0,
+"bps_wr": 0,
+"write_threshold": 0,
+"encrypted": false,
+"bps": 0,
+"bps_rd": 0,
+"cache": {
+"no-flush": false,
+"direct": false,
+"writeback": true
+},
+"file": "/tmp/pull4.qcow2",
+"encryption_key_missing": false
+}
+]
diff --git a/tests/qemublocktestdata/bitmap/synthetic.out
b/tests/qemublocktestdata/bitmap/synthetic.out
new file mode 100644
index 00..d941716d4b
--- /dev/null
+++ b/tests/qemublocktestdata/bitmap/synthetic.out
@@ -0,0 +1,6 @@
+libvirt-1-format:
+ current: recod:1 busy:0 persist:1 inconist:0 gran:65536 dirty:0
+d: recod:0 busy:0 persist:0 inconist:0 gran:65536 dirty:0
+c: recod:0 busy:0 persist:1 inconist:0 gran:1234 dirty:0
+b: recod:0 busy:1 persist:1 inconist:0 gran:65536 dirty:21314
+a: recod:0 busy:0 persist:1 inconist:1 gran:65536 d
[libvirt] [PATCH 13/19] qemu: backup: Extract calculations of bitmaps to merge for incremental backup
Separate the for now incomplete code that collects the bitmaps to be
merged for an incremental backup into a separate function. This will
allow to add testing prior to the improvement of the algorithm to
include snapshots.
Signed-off-by: Peter Krempa
---
src/qemu/qemu_backup.c | 37 +
1 file changed, 25 insertions(+), 12 deletions(-)
diff --git a/src/qemu/qemu_backup.c b/src/qemu/qemu_backup.c
index 1cd466d211..31949b5399 100644
--- a/src/qemu/qemu_backup.c
+++ b/src/qemu/qemu_backup.c
@@ -170,30 +170,43 @@ qemuBackupDiskDataCleanup(virDomainObjPtr vm,
}
-
-static int
-qemuBackupDiskPrepareOneBitmaps(struct qemuBackupDiskData *dd,
-virJSONValuePtr actions,
-virDomainMomentDefPtr *incremental)
+static virJSONValuePtr
+qemuBackupDiskPrepareOneBitmapsChain(virDomainMomentDefPtr *incremental,
+ virStorageSourcePtr backingChain)
{
-g_autoptr(virJSONValue) mergebitmaps = NULL;
-g_autoptr(virJSONValue) mergebitmapsstore = NULL;
+g_autoptr(virJSONValue) ret = NULL;
-if (!(mergebitmaps = virJSONValueNewArray()))
-return -1;
+if (!(ret = virJSONValueNewArray()))
+return NULL;
/* TODO: this code works only if the bitmaps are present on a single node.
* The algorithm needs to be changed so that it looks into the backing
chain
* so that we can combine all relevant bitmaps for a given backing chain */
while (*incremental) {
-if (qemuMonitorTransactionBitmapMergeSourceAddBitmap(mergebitmaps,
-
dd->domdisk->src->nodeformat,
+if (qemuMonitorTransactionBitmapMergeSourceAddBitmap(ret,
+
backingChain->nodeformat,
(*incremental)->name) < 0)
-return -1;
+return NULL;
incremental++;
}
+return g_steal_pointer(&ret);
+}
+
+
+static int
+qemuBackupDiskPrepareOneBitmaps(struct qemuBackupDiskData *dd,
+virJSONValuePtr actions,
+virDomainMomentDefPtr *incremental)
+{
+g_autoptr(virJSONValue) mergebitmaps = NULL;
+g_autoptr(virJSONValue) mergebitmapsstore = NULL;
+
+if (!(mergebitmaps = qemuBackupDiskPrepareOneBitmapsChain(incremental,
+
dd->domdisk->src)))
+return -1;
+
if (!(mergebitmapsstore = virJSONValueCopy(mergebitmaps)))
return -1;
--
2.23.0
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 11/19] tests: qemublock: Add test case for detecting bitmaps as we create snapshots
Add test data gathered from a run of qemu after creating bitmaps and
snapshots together in various combinations.
Signed-off-by: Peter Krempa
---
tests/qemublocktest.c | 1 +
tests/qemublocktestdata/bitmap/snapshots.json | 836 ++
tests/qemublocktestdata/bitmap/snapshots.out | 14 +
3 files changed, 851 insertions(+)
create mode 100644 tests/qemublocktestdata/bitmap/snapshots.json
create mode 100644 tests/qemublocktestdata/bitmap/snapshots.out
diff --git a/tests/qemublocktest.c b/tests/qemublocktest.c
index e3aee03c81..25afef46bb 100644
--- a/tests/qemublocktest.c
+++ b/tests/qemublocktest.c
@@ -777,6 +777,7 @@ mymain(void)
TEST_BITMAP_DETECT("basic");
TEST_BITMAP_DETECT("synthetic");
+TEST_BITMAP_DETECT("snapshots");
cleanup:
virHashFree(diskxmljsondata.schema);
diff --git a/tests/qemublocktestdata/bitmap/snapshots.json
b/tests/qemublocktestdata/bitmap/snapshots.json
new file mode 100644
index 00..87e77ad408
--- /dev/null
+++ b/tests/qemublocktestdata/bitmap/snapshots.json
@@ -0,0 +1,836 @@
+[
+{
+"iops_rd": 0,
+"detect_zeroes": "off",
+"image": {
+"backing-image": {
+"backing-image": {
+"backing-image": {
+"backing-image": {
+"virtual-size": 10485760,
+"filename": "/tmp/pull4.qcow2",
+"cluster-size": 65536,
+"format": "qcow2",
+"actual-size": 208896,
+"format-specific": {
+"type": "qcow2",
+"data": {
+"compat": "1.1",
+"lazy-refcounts": false,
+"bitmaps": [
+{
+"flags": [
+"auto"
+],
+"name": "a",
+"granularity": 65536
+}
+],
+"refcount-bits": 16,
+"corrupt": false
+}
+},
+"dirty-flag": false
+},
+"backing-filename-format": "qcow2",
+"virtual-size": 10485760,
+"filename": "/tmp/pull4.1575911522",
+"cluster-size": 65536,
+"format": "qcow2",
+"actual-size": 208896,
+"format-specific": {
+"type": "qcow2",
+"data": {
+"compat": "1.1",
+"lazy-refcounts": false,
+"bitmaps": [
+{
+"flags": [
+"auto"
+],
+"name": "a",
+"granularity": 65536
+}
+],
+"refcount-bits": 16,
+"corrupt": false
+}
+},
+"full-backing-filename": "/tmp/pull4.qcow2",
+"backing-filename": "/tmp/pull4.qcow2",
+"dirty-flag": false
+},
+"backing-filename-format": "qcow2",
+"virtual-size": 10485760,
+"filename": "/tmp/pull4.1575911527",
+"cluster-size": 65536,
+"format": "qcow2",
+"actual-size": 217088,
+"format-specific": {
+"type": "qcow2",
+"data": {
+"compat": "1.1",
+"lazy-refcounts": false,
+"bitmaps": [
+{
+"flags": [
+"auto"
+],
+"name": "c",
+"granularity": 65536
+},
+{
+"flags": [
+
+],
+"name": "b",
+"granula
[libvirt] [PATCH 12/19] qemu: backup: Return 'def' instead of 'obj' from qemuBackupBeginCollectIncrementalCheckpoints
The object itself has no extra value and it would make testing the code
harder. Refactor it to remove just the definition pointer.
Signed-off-by: Peter Krempa
---
src/qemu/qemu_backup.c | 29 -
1 file changed, 16 insertions(+), 13 deletions(-)
diff --git a/src/qemu/qemu_backup.c b/src/qemu/qemu_backup.c
index 5c9747f09d..1cd466d211 100644
--- a/src/qemu/qemu_backup.c
+++ b/src/qemu/qemu_backup.c
@@ -174,7 +174,7 @@ qemuBackupDiskDataCleanup(virDomainObjPtr vm,
static int
qemuBackupDiskPrepareOneBitmaps(struct qemuBackupDiskData *dd,
virJSONValuePtr actions,
-virDomainMomentObjPtr *incremental)
+virDomainMomentDefPtr *incremental)
{
g_autoptr(virJSONValue) mergebitmaps = NULL;
g_autoptr(virJSONValue) mergebitmapsstore = NULL;
@@ -188,7 +188,7 @@ qemuBackupDiskPrepareOneBitmaps(struct qemuBackupDiskData
*dd,
while (*incremental) {
if (qemuMonitorTransactionBitmapMergeSourceAddBitmap(mergebitmaps,
dd->domdisk->src->nodeformat,
-
(*incremental)->def->name) < 0)
+
(*incremental)->name) < 0)
return -1;
incremental++;
@@ -232,7 +232,7 @@ qemuBackupDiskPrepareDataOne(virDomainObjPtr vm,
virDomainBackupDiskDefPtr backupdisk,
struct qemuBackupDiskData *dd,
virJSONValuePtr actions,
- virDomainMomentObjPtr *incremental,
+ virDomainMomentDefPtr *incremental,
virQEMUDriverConfigPtr cfg,
bool removeStore)
{
@@ -323,7 +323,7 @@ qemuBackupDiskPrepareDataOnePull(virJSONValuePtr actions,
static ssize_t
qemuBackupDiskPrepareData(virDomainObjPtr vm,
virDomainBackupDefPtr def,
- virDomainMomentObjPtr *incremental,
+ virDomainMomentDefPtr *incremental,
virJSONValuePtr actions,
virQEMUDriverConfigPtr cfg,
struct qemuBackupDiskData **rdd,
@@ -505,24 +505,27 @@ qemuBackupBeginPullExportDisks(virDomainObjPtr vm,
* @vm: domain object
* @incrFrom: name of checkpoint representing starting point of incremental
backup
*
- * Returns a NULL terminated list of pointers to checkpoints in chronological
- * order starting from the 'current' checkpoint until reaching @incrFrom.
+ * Returns a NULL terminated list of pointers to checkpoint definitions in
+ * chronological order starting from the 'current' checkpoint until reaching
+ * @incrFrom.
*/
-static virDomainMomentObjPtr *
+static virDomainMomentDefPtr *
qemuBackupBeginCollectIncrementalCheckpoints(virDomainObjPtr vm,
const char *incrFrom)
{
virDomainMomentObjPtr n = virDomainCheckpointGetCurrent(vm->checkpoints);
-g_autofree virDomainMomentObjPtr *incr = NULL;
+g_autofree virDomainMomentDefPtr *incr = NULL;
size_t nincr = 0;
while (n) {
-if (VIR_APPEND_ELEMENT_COPY(incr, nincr, n) < 0)
+virDomainMomentDefPtr def = n->def;
+
+if (VIR_APPEND_ELEMENT_COPY(incr, nincr, def) < 0)
return NULL;
-if (STREQ(n->def->name, incrFrom)) {
-virDomainMomentObjPtr terminator = NULL;
-if (VIR_APPEND_ELEMENT_COPY(incr, nincr, terminator) < 0)
+if (STREQ(def->name, incrFrom)) {
+def = NULL;
+if (VIR_APPEND_ELEMENT_COPY(incr, nincr, def) < 0)
return NULL;
return g_steal_pointer(&incr);
@@ -648,7 +651,7 @@ qemuBackupBegin(virDomainObjPtr vm,
bool pull = false;
virDomainMomentObjPtr chk = NULL;
g_autoptr(virDomainCheckpointDef) chkdef = NULL;
-g_autofree virDomainMomentObjPtr *incremental = NULL;
+g_autofree virDomainMomentDefPtr *incremental = NULL;
g_autoptr(virJSONValue) actions = NULL;
struct qemuBackupDiskData *dd = NULL;
ssize_t ndd = 0;
--
2.23.0
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 03/19] qemu: monitor: Extract data about dirty-bimaps in qemuMonitorBlockGetNamedNodeData
We will need to inspect the presence and attributes for dirty bitmaps.
Extract them when processing reply of query-named-block-nodes.
Signed-off-by: Peter Krempa
---
src/qemu/qemu_monitor.h | 15
src/qemu/qemu_monitor_json.c | 74
2 files changed, 89 insertions(+)
diff --git a/src/qemu/qemu_monitor.h b/src/qemu/qemu_monitor.h
index 79e078fca4..1c990923d6 100644
--- a/src/qemu/qemu_monitor.h
+++ b/src/qemu/qemu_monitor.h
@@ -676,12 +676,27 @@ int
qemuMonitorBlockStatsUpdateCapacityBlockdev(qemuMonitorPtr mon,
virHashTablePtr stats)
ATTRIBUTE_NONNULL(2);
+typedef struct _qemuBlockNamedNodeDataBitmap qemuBlockNamedNodeDataBitmap;
+typedef qemuBlockNamedNodeDataBitmap *qemuBlockNamedNodeDataBitmapPtr;
+struct _qemuBlockNamedNodeDataBitmap {
+char *name;
+bool recording;
+bool busy;
+bool persistent;
+bool inconsistent;
+
+unsigned long long dirtybytes;
+unsigned long long granularity;
+};
typedef struct _qemuBlockNamedNodeData qemuBlockNamedNodeData;
typedef qemuBlockNamedNodeData *qemuBlockNamedNodeDataPtr;
struct _qemuBlockNamedNodeData {
unsigned long long capacity;
unsigned long long physical;
+
+qemuBlockNamedNodeDataBitmapPtr *bitmaps;
+size_t nbitmaps;
};
virHashTablePtr
diff --git a/src/qemu/qemu_monitor_json.c b/src/qemu/qemu_monitor_json.c
index 00e1d3ce15..e3a6e3a6a2 100644
--- a/src/qemu/qemu_monitor_json.c
+++ b/src/qemu/qemu_monitor_json.c
@@ -2873,14 +2873,84 @@
qemuMonitorJSONBlockStatsUpdateCapacityBlockdev(qemuMonitorPtr mon,
}
+static void
+qemuMonitorJSONBlockNamedNodeDataBitmapFree(qemuBlockNamedNodeDataBitmapPtr
bitmap)
+{
+if (!bitmap)
+return;
+
+g_free(bitmap->name);
+g_free(bitmap);
+}
+
+G_DEFINE_AUTOPTR_CLEANUP_FUNC(qemuBlockNamedNodeDataBitmap,
+ qemuMonitorJSONBlockNamedNodeDataBitmapFree);
+
+
static void
qemuMonitorJSONBlockNamedNodeDataFree(qemuBlockNamedNodeDataPtr data)
{
+size_t i;
+
+if (!data)
+return;
+
+for (i = 0; i < data->nbitmaps; i++)
+qemuMonitorJSONBlockNamedNodeDataBitmapFree(data->bitmaps[i]);
+g_free(data->bitmaps);
g_free(data);
}
G_DEFINE_AUTOPTR_CLEANUP_FUNC(qemuBlockNamedNodeData,
qemuMonitorJSONBlockNamedNodeDataFree);
+static qemuBlockNamedNodeDataBitmapPtr
+qemuMonitorJSONBlockGetNamedNodeDataBitmapOne(virJSONValuePtr val)
+{
+g_autoptr(qemuBlockNamedNodeDataBitmap) bitmap = NULL;
+const char *name;
+
+bitmap = g_new0(qemuBlockNamedNodeDataBitmap, 1);
+
+if (!(name = virJSONValueObjectGetString(val, "name")))
+return NULL;
+
+bitmap->name = g_strdup(name);
+
+ignore_value(virJSONValueObjectGetBoolean(val, "recording",
&bitmap->recording));
+ignore_value(virJSONValueObjectGetBoolean(val, "persistent",
&bitmap->persistent));
+ignore_value(virJSONValueObjectGetBoolean(val, "busy", &bitmap->busy));
+ignore_value(virJSONValueObjectGetBoolean(val, "inconsistent",
&bitmap->inconsistent));
+ignore_value(virJSONValueObjectGetNumberUlong(val, "granularity",
&bitmap->granularity));
+ignore_value(virJSONValueObjectGetNumberUlong(val, "count",
&bitmap->dirtybytes));
+
+return g_steal_pointer(&bitmap);
+}
+
+
+static void
+qemuMonitorJSONBlockGetNamedNodeDataBitmaps(virJSONValuePtr bitmaps,
+qemuBlockNamedNodeDataPtr data)
+{
+size_t nbitmaps = virJSONValueArraySize(bitmaps);
+size_t i;
+
+data->bitmaps = g_new0(qemuBlockNamedNodeDataBitmapPtr, nbitmaps);
+
+for (i = 0; i < nbitmaps; i++) {
+virJSONValuePtr bitmap = virJSONValueArrayGet(bitmaps, i);
+qemuBlockNamedNodeDataBitmapPtr tmp;
+
+if (!bitmap)
+continue;
+
+if (!(tmp = qemuMonitorJSONBlockGetNamedNodeDataBitmapOne(bitmap)))
+continue;
+
+data->bitmaps[data->nbitmaps++] = tmp;
+}
+}
+
+
static int
qemuMonitorJSONBlockGetNamedNodeDataWorker(size_t pos G_GNUC_UNUSED,
virJSONValuePtr val,
@@ -2888,6 +2958,7 @@ qemuMonitorJSONBlockGetNamedNodeDataWorker(size_t pos
G_GNUC_UNUSED,
{
virHashTablePtr nodes = opaque;
virJSONValuePtr img;
+virJSONValuePtr bitmaps;
const char *nodename;
g_autoptr(qemuBlockNamedNodeData) ent = NULL;
@@ -2904,6 +2975,9 @@ qemuMonitorJSONBlockGetNamedNodeDataWorker(size_t pos
G_GNUC_UNUSED,
if (virJSONValueObjectGetNumberUlong(img, "actual-size", &ent->physical) <
0)
ent->physical = ent->capacity;
+if ((bitmaps = virJSONValueObjectGetArray(val, "dirty-bitmaps")))
+qemuMonitorJSONBlockGetNamedNodeDataBitmaps(bitmaps, ent);
+
if (virHashAddEntry(nodes, nodename, ent) < 0)
return -1;
--
2.23.0
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 05/19] tests: qemublock: Add test for bitmap detection
Test the extraction of data about changed block tracking bitmaps. The
first test case adds a simple scenario of multiple bitmaps in one layer.
The test data will be also later reused for testing the code that
determines which bitmaps to merge for an incremental backup.
Signed-off-by: Peter Krempa
---
tests/qemublocktest.c | 75 ++
tests/qemublocktestdata/bitmap/basic.json | 117 ++
tests/qemublocktestdata/bitmap/basic.out | 6 ++
3 files changed, 198 insertions(+)
create mode 100644 tests/qemublocktestdata/bitmap/basic.json
create mode 100644 tests/qemublocktestdata/bitmap/basic.out
diff --git a/tests/qemublocktest.c b/tests/qemublocktest.c
index 2c170548ec..16bee47a12 100644
--- a/tests/qemublocktest.c
+++ b/tests/qemublocktest.c
@@ -27,6 +27,7 @@
# include "virlog.h"
# include "qemu/qemu_block.h"
# include "qemu/qemu_qapi.h"
+# include "qemu/qemu_monitor_json.h"
# include "qemu/qemu_command.h"
@@ -492,6 +493,71 @@ testQemuDiskXMLToPropsValidateFileSrcOnly(const void
*opaque)
}
+static const char *bitmapDetectPrefix = "qemublocktestdata/bitmap/";
+
+static void
+testQemuDetectBitmapsWorker(virHashTablePtr nodedata,
+const char *nodename,
+virBufferPtr buf)
+{
+qemuBlockNamedNodeDataPtr data;
+size_t i;
+
+if (!(data = virHashLookup(nodedata, nodename)))
+return;
+
+virBufferAsprintf(buf, "%s:\n", nodename);
+virBufferAdjustIndent(buf, 1);
+
+for (i = 0; i < data->nbitmaps; i++) {
+qemuBlockNamedNodeDataBitmapPtr bitmap = data->bitmaps[i];
+
+virBufferAsprintf(buf, "%8s: recod:%d busy:%d persist:%d inconist:%d
gran:%llu dirty:%llu\n",
+ bitmap->name, bitmap->recording, bitmap->busy,
+ bitmap->persistent, bitmap->inconsistent,
+ bitmap->granularity, bitmap->dirtybytes);
+}
+
+virBufferAdjustIndent(buf, -1);
+}
+
+
+static int
+testQemuDetectBitmaps(const void *opaque)
+{
+const char *name = opaque;
+g_autoptr(virJSONValue) nodedatajson = NULL;
+g_autoptr(virHashTable) nodedata = NULL;
+g_autofree char *actual = NULL;
+g_autofree char *expectpath = NULL;
+g_auto(virBuffer) buf = VIR_BUFFER_INITIALIZER;
+size_t i;
+
+expectpath = g_strdup_printf("%s/%s%s.out", abs_srcdir,
+ bitmapDetectPrefix, name);
+
+if (!(nodedatajson = virTestLoadFileJSON(bitmapDetectPrefix, name,
+ ".json", NULL)))
+return -1;
+
+if (!(nodedata = qemuMonitorJSONBlockGetNamedNodeDataJSON(nodedatajson))) {
+VIR_TEST_VERBOSE("failed to load nodedata JSON\n");
+return -1;
+}
+
+/* we detect for the first 30 nodenames for simplicity */
+for (i = 0; i < 30; i++) {
+g_autofree char *nodename = g_strdup_printf("libvirt-%zu-format", i);
+
+testQemuDetectBitmapsWorker(nodedata, nodename, &buf);
+}
+
+actual = virBufferContentAndReset(&buf);
+
+return virTestCompareToFile(actual, expectpath);
+}
+
+
static int
mymain(void)
{
@@ -702,6 +768,15 @@ mymain(void)
TEST_IMAGE_CREATE("network-ssh-qcow2", NULL);
TEST_IMAGE_CREATE("network-sheepdog-qcow2", NULL);
+# define TEST_BITMAP_DETECT(testname) \
+do { \
+if (virTestRun("bitmap detect " testname, \
+ testQemuDetectBitmaps, testname) < 0) \
+ret = -1; \
+} while (0)
+
+TEST_BITMAP_DETECT("basic");
+
cleanup:
virHashFree(diskxmljsondata.schema);
qemuTestDriverFree(&driver);
diff --git a/tests/qemublocktestdata/bitmap/basic.json
b/tests/qemublocktestdata/bitmap/basic.json
new file mode 100644
index 00..9d418b1a37
--- /dev/null
+++ b/tests/qemublocktestdata/bitmap/basic.json
@@ -0,0 +1,117 @@
+[
+{
+"iops_rd": 0,
+"detect_zeroes": "off",
+"image": {
+"virtual-size": 10485760,
+"filename": "/tmp/pull4.qcow2",
+"cluster-size": 65536,
+"format": "qcow2",
+"actual-size": 200704,
+"format-specific": {
+"type": "qcow2",
+"data": {
+"compat": "1.1",
+"lazy-refcounts": false,
+"refcount-bits": 16,
+"corrupt": false
+}
+},
+"dirty-flag": false
+},
+"iops_wr": 0,
+"ro": false,
+"node-name": "libvirt-1-format",
+"backing_file_depth": 0,
+"drv": "qcow2",
+"iops": 0,
+"bps_wr": 0,
+"write_threshold": 0,
+"dirty-bitmaps": [
+{
+"name": "current",
+"recording": true,
+"persistent": true,
+"busy": false,
+"status": "active",
+"granularity": 65536,
+"co
[libvirt] [PATCH 04/19] qemu: monitor: Extract internals of qemuMonitorJSONBlockGetNamedNodeData
For testing purposes it will be beneficial to be able to parse the data
from JSON directly rather than trying to simulate the monitor. Extract
the worker bits and export them.
Signed-off-by: Peter Krempa
---
src/qemu/qemu_monitor_json.c | 18 +-
src/qemu/qemu_monitor_json.h | 3 +++
2 files changed, 16 insertions(+), 5 deletions(-)
diff --git a/src/qemu/qemu_monitor_json.c b/src/qemu/qemu_monitor_json.c
index e3a6e3a6a2..856c2c2778 100644
--- a/src/qemu/qemu_monitor_json.c
+++ b/src/qemu/qemu_monitor_json.c
@@ -2993,14 +2993,10 @@ qemuMonitorJSONBlockGetNamedNodeDataWorker(size_t pos
G_GNUC_UNUSED,
virHashTablePtr
-qemuMonitorJSONBlockGetNamedNodeData(qemuMonitorPtr mon)
+qemuMonitorJSONBlockGetNamedNodeDataJSON(virJSONValuePtr nodes)
{
-g_autoptr(virJSONValue) nodes = NULL;
g_autoptr(virHashTable) ret = NULL;
-if (!(nodes = qemuMonitorJSONQueryNamedBlockNodes(mon)))
-return NULL;
-
if (!(ret = virHashNew((virHashDataFree)
qemuMonitorJSONBlockNamedNodeDataFree)))
return NULL;
@@ -3013,6 +3009,18 @@ qemuMonitorJSONBlockGetNamedNodeData(qemuMonitorPtr mon)
}
+virHashTablePtr
+qemuMonitorJSONBlockGetNamedNodeData(qemuMonitorPtr mon)
+{
+g_autoptr(virJSONValue) nodes = NULL;
+
+if (!(nodes = qemuMonitorJSONQueryNamedBlockNodes(mon)))
+return NULL;
+
+return qemuMonitorJSONBlockGetNamedNodeDataJSON(nodes);
+}
+
+
int qemuMonitorJSONBlockResize(qemuMonitorPtr mon,
const char *device,
const char *nodename,
diff --git a/src/qemu/qemu_monitor_json.h b/src/qemu/qemu_monitor_json.h
index 5d05772fa2..44926464b9 100644
--- a/src/qemu/qemu_monitor_json.h
+++ b/src/qemu/qemu_monitor_json.h
@@ -87,6 +87,9 @@ int qemuMonitorJSONBlockStatsUpdateCapacity(qemuMonitorPtr
mon,
int qemuMonitorJSONBlockStatsUpdateCapacityBlockdev(qemuMonitorPtr mon,
virHashTablePtr stats);
+virHashTablePtr
+qemuMonitorJSONBlockGetNamedNodeDataJSON(virJSONValuePtr nodes);
+
virHashTablePtr
qemuMonitorJSONBlockGetNamedNodeData(qemuMonitorPtr mon);
--
2.23.0
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 08/19] qemu: snapshot: Fold formatting of snapshot transaction into prepare func
qemuDomainSnapshotDiskPrepareOne is already called for each disk which
is member of the snapshot so we don't need to iterate through the
snapshot list again to generate members of the 'transaction' command for
each snapshot.
Signed-off-by: Peter Krempa
---
src/qemu/qemu_driver.c | 45 +-
1 file changed, 18 insertions(+), 27 deletions(-)
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index 41a124d215..d2769dab1a 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -15285,7 +15285,8 @@ qemuDomainSnapshotDiskPrepareOne(virQEMUDriverPtr
driver,
virHashTablePtr blockNamedNodeData,
bool reuse,
bool blockdev,
- qemuDomainAsyncJob asyncJob)
+ qemuDomainAsyncJob asyncJob,
+ virJSONValuePtr actions)
{
virDomainDiskDefPtr persistdisk;
bool supportsCreate;
@@ -15358,10 +15359,17 @@ qemuDomainSnapshotDiskPrepareOne(virQEMUDriverPtr
driver,
dd->prepared = true;
-if (blockdev &&
-qemuDomainSnapshotDiskPrepareOneBlockdev(driver, vm, dd, cfg, reuse,
- blockNamedNodeData, asyncJob)
< 0)
-return -1;
+if (blockdev) {
+if (qemuDomainSnapshotDiskPrepareOneBlockdev(driver, vm, dd, cfg,
reuse,
+ blockNamedNodeData,
asyncJob) < 0)
+return -1;
+
+if (qemuBlockSnapshotAddBlockdev(actions, dd->disk, dd->src) < 0)
+return -1;
+} else {
+if (qemuBlockSnapshotAddLegacy(actions, dd->disk, dd->src, reuse) < 0)
+return -1;
+}
return 0;
}
@@ -15383,7 +15391,8 @@ qemuDomainSnapshotDiskPrepare(virQEMUDriverPtr driver,
virHashTablePtr blockNamedNodeData,
qemuDomainAsyncJob asyncJob,
qemuDomainSnapshotDiskDataPtr *rdata,
- size_t *rndata)
+ size_t *rndata,
+ virJSONValuePtr actions)
{
size_t i;
qemuDomainSnapshotDiskDataPtr data;
@@ -15403,7 +15412,8 @@ qemuDomainSnapshotDiskPrepare(virQEMUDriverPtr driver,
data + ndata++,
blockNamedNodeData,
reuse, blockdev,
- asyncJob) < 0)
+ asyncJob,
+ actions) < 0)
goto cleanup;
}
@@ -15516,7 +15526,7 @@ qemuDomainSnapshotCreateDiskActive(virQEMUDriverPtr
driver,
* have to roll back later */
if (qemuDomainSnapshotDiskPrepare(driver, vm, snap, cfg, reuse, blockdev,
blockNamedNodeData, asyncJob,
- &diskdata, &ndiskdata) < 0)
+ &diskdata, &ndiskdata, actions) < 0)
goto cleanup;
/* check whether there's anything to do */
@@ -15525,25 +15535,6 @@ qemuDomainSnapshotCreateDiskActive(virQEMUDriverPtr
driver,
goto cleanup;
}
- /* Based on earlier qemuDomainSnapshotPrepare, all disks in this list are
- * now either VIR_DOMAIN_SNAPSHOT_LOCATION_NONE, or
- * VIR_DOMAIN_SNAPSHOT_LOCATION_EXTERNAL with a valid file name and
- * qcow2 format. */
-for (i = 0; i < ndiskdata; i++) {
-if (blockdev) {
-if (qemuBlockSnapshotAddBlockdev(actions,
- diskdata[i].disk,
- diskdata[i].src) < 0)
-goto cleanup;
-} else {
-if (qemuBlockSnapshotAddLegacy(actions,
- diskdata[i].disk,
- diskdata[i].src,
- reuse) < 0)
-goto cleanup;
-}
-}
-
if (qemuDomainObjEnterMonitorAsync(driver, vm, asyncJob) < 0)
goto cleanup;
--
2.23.0
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 17/19] qemu: block: Introduce qemuBlockNamedNodeDataGetBitmapByName
This function looks up a named bitmap for a virStorageSource in the data
returned from query-named-block-nodes.
Signed-off-by: Peter Krempa
---
src/qemu/qemu_block.c | 32
src/qemu/qemu_block.h | 5 +
2 files changed, 37 insertions(+)
diff --git a/src/qemu/qemu_block.c b/src/qemu/qemu_block.c
index ada2c52947..629a09b897 100644
--- a/src/qemu/qemu_block.c
+++ b/src/qemu/qemu_block.c
@@ -2612,3 +2612,35 @@ qemuBlockRemoveImageMetadata(virQEMUDriverPtr driver,
return ret;
}
+
+
+/**
+ * qemuBlockNamedNodeDataGetBitmapByName:
+ * @blockNamedNodeData: hash table returned by qemuMonitorBlockGetNamedNodeData
+ * @src: disk source to find the bitmap for
+ * @bitmap: name of the bitmap to find
+ *
+ * Looks up a bitmap named @bitmap of the @src image.
+ */
+qemuBlockNamedNodeDataBitmapPtr
+qemuBlockNamedNodeDataGetBitmapByName(virHashTablePtr blockNamedNodeData,
+ virStorageSourcePtr src,
+ const char *bitmap)
+{
+qemuBlockNamedNodeDataPtr nodedata;
+size_t i;
+
+if (!(nodedata = virHashLookup(blockNamedNodeData, src->nodeformat)))
+return NULL;
+
+for (i = 0; i < nodedata->nbitmaps; i++) {
+qemuBlockNamedNodeDataBitmapPtr bitmapdata = nodedata->bitmaps[i];
+
+if (STRNEQ(bitmapdata->name, bitmap))
+continue;
+
+return bitmapdata;
+}
+
+return NULL;
+}
diff --git a/src/qemu/qemu_block.h b/src/qemu/qemu_block.h
index 5854641027..1a38e0eccf 100644
--- a/src/qemu/qemu_block.h
+++ b/src/qemu/qemu_block.h
@@ -203,3 +203,8 @@ qemuBlockRemoveImageMetadata(virQEMUDriverPtr driver,
virDomainObjPtr vm,
const char *diskTarget,
virStorageSourcePtr src);
+
+qemuBlockNamedNodeDataBitmapPtr
+qemuBlockNamedNodeDataGetBitmapByName(virHashTablePtr blockNamedNodeData,
+ virStorageSourcePtr src,
+ const char *bitmap);
--
2.23.0
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 10/19] qemu: snapshot: Propagate active bitmaps through external snapshots
Re-create any active persistent bitmap in the snapshot overlay image so
that tracking for a checkpoint is persisted. While this basically
duplicates data in the allocation map it's currently the only possible
way as qemu can't mirror the allocation map into a dirty bitmap if we'd
ever want to do a backup.
Signed-off-by: Peter Krempa
---
src/qemu/qemu_driver.c | 39 +++
1 file changed, 39 insertions(+)
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index d2769dab1a..8ccd6b7c97 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -15223,6 +15223,42 @@
qemuDomainSnapshotDiskCleanup(qemuDomainSnapshotDiskDataPtr data,
}
+/**
+ * qemuDomainSnapshotDiskBitmapsPropagate:
+ *
+ * This function propagates any active persistent bitmap present in the
original
+ * image into the new snapshot. We leave the original bitmap active as in cases
+ * when the overlay is discarded (snapshot revert with abandoning the history)
+ * everything works as expected.
+ * */
+static int
+qemuDomainSnapshotDiskBitmapsPropagate(qemuDomainSnapshotDiskDataPtr dd,
+ virJSONValuePtr actions,
+ virHashTablePtr blockNamedNodeData)
+{
+qemuBlockNamedNodeDataPtr entry;
+size_t i;
+
+if (!(entry = virHashLookup(blockNamedNodeData,
dd->disk->src->nodeformat)))
+return 0;
+
+for (i = 0; i < entry->nbitmaps; i++) {
+qemuBlockNamedNodeDataBitmapPtr bitmap = entry->bitmaps[i];
+
+/* we don't care about temporary, inconsistent, or disabled bitmaps */
+if (!bitmap->persistent || !bitmap->recording || bitmap->inconsistent)
+continue;
+
+if (qemuMonitorTransactionBitmapAdd(actions, dd->src->nodeformat,
+bitmap->name, true, false,
+bitmap->granularity) < 0)
+return -1;
+}
+
+return 0;
+}
+
+
static int
qemuDomainSnapshotDiskPrepareOneBlockdev(virQEMUDriverPtr driver,
virDomainObjPtr vm,
@@ -15364,6 +15400,9 @@ qemuDomainSnapshotDiskPrepareOne(virQEMUDriverPtr
driver,
blockNamedNodeData,
asyncJob) < 0)
return -1;
+if (qemuDomainSnapshotDiskBitmapsPropagate(dd, actions,
blockNamedNodeData) < 0)
+return -1;
+
if (qemuBlockSnapshotAddBlockdev(actions, dd->disk, dd->src) < 0)
return -1;
} else {
--
2.23.0
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 01/19] virsh: Add QMP command wrapping for 'qemu-monitor-command'
Issuing simple QMP commands is pain as they need to be wrapped by the
JSON wrapper:
{ "execute": "COMMAND" }
and optionally also:
{ "execute": "COMMAND", "arguments":...}
For simple commands without arguments we can add syntax sugar to virsh
which allows simple usage of QMP and additionally prepares also for
passing through of the 'arguments' section.
Signed-off-by: Peter Krempa
---
docs/manpages/virsh.rst | 24 +---
tools/virsh-domain.c| 32 +---
2 files changed, 46 insertions(+), 10 deletions(-)
diff --git a/docs/manpages/virsh.rst b/docs/manpages/virsh.rst
index e9d6deaee1..697f83e5b2 100644
--- a/docs/manpages/virsh.rst
+++ b/docs/manpages/virsh.rst
@@ -7434,16 +7434,26 @@ qemu-monitor-command
.. code-block:: shell
- qemu-monitor-command domain { [--hmp] | [--pretty] } command...
+ qemu-monitor-command domain { [--hmp] | [--qmp] [--pretty] } command...
Send an arbitrary monitor command *command* to domain *domain* through the
-qemu monitor. The results of the command will be printed on stdout. If
-*--hmp* is passed, the command is considered to be a human monitor command
+qemu monitor. The results of the command will be printed on stdout.
+
+*command* is directly passed to qemu. If more than one argument is provided for
+*command*, they are concatenated with a space in between before passing the
+single command to the monitor.
+
+If *--qmp* is passed the first argument passed as *command* is used as a QMP
+command name and appropriately wrapped into a JSON block. Additionally a second
+argument passed as *command* is appended as value of the 'arguments' parameter
+of the QMP command verbatim.
+
+If *--pretty* is given, and the monitor uses QMP, then the output will be
+pretty-printed.
+
+If *--hmp* is passed, the command is considered to be a human monitor command
and libvirt will automatically convert it into QMP if needed. In that case
-the result will also be converted back from QMP. If *--pretty* is given,
-and the monitor uses QMP, then the output will be pretty-printed. If more
-than one argument is provided for *command*, they are concatenated with a
-space in between before passing the single command to the monitor.
+the result will also be converted back from QMP.
qemu-agent-command
diff --git a/tools/virsh-domain.c b/tools/virsh-domain.c
index 56137bdd74..9447fa2904 100644
--- a/tools/virsh-domain.c
+++ b/tools/virsh-domain.c
@@ -9522,6 +9522,10 @@ static const vshCmdOptDef opts_qemu_monitor_command[] = {
.type = VSH_OT_BOOL,
.help = N_("pretty-print any qemu monitor protocol output")
},
+{.name = "qmp",
+ .type = VSH_OT_BOOL,
+ .help = N_("wrap the 'cmd' argument in JSON wrapper for QMP")
+},
{.name = "cmd",
.type = VSH_OT_ARGV,
.flags = VSH_OFLAG_REQ,
@@ -9539,16 +9543,38 @@ cmdQemuMonitorCommand(vshControl *ctl, const vshCmd
*cmd)
unsigned int flags = 0;
const vshCmdOpt *opt = NULL;
virBuffer buf = VIR_BUFFER_INITIALIZER;
+bool qmp = vshCommandOptBool(cmd, "qmp");
VSH_EXCLUSIVE_OPTIONS("hmp", "pretty");
+VSH_EXCLUSIVE_OPTIONS("hmp", "qmp");
if (!(dom = virshCommandOptDomain(ctl, cmd, NULL)))
return false;
-while ((opt = vshCommandOptArgv(ctl, cmd, opt)))
-virBufferAsprintf(&buf, "%s ", opt->data);
+if (qmp) {
+const char *command = NULL;
+const char *arguments = NULL;
-virBufferTrim(&buf, " ", -1);
+if ((opt = vshCommandOptArgv(ctl, cmd, opt)))
+command = opt->data;
+if ((opt = vshCommandOptArgv(ctl, cmd, opt)))
+arguments = opt->data;
+
+if (!command || (arguments && vshCommandOptArgv(ctl, cmd, opt))) {
+vshError(ctl, "%s", _("-qmp option requires 1 or 2 arguments"));
+return false;
+}
+
+virBufferAsprintf(&buf, "{\"execute\":\"%s\"", command);
+if (arguments)
+virBufferAsprintf(&buf, ", \"arguments\":%s", arguments);
+virBufferAddLit(&buf, "}");
+} else {
+while ((opt = vshCommandOptArgv(ctl, cmd, opt)))
+virBufferAsprintf(&buf, "%s ", opt->data);
+
+virBufferTrim(&buf, " ", -1);
+}
monitor_cmd = virBufferContentAndReset(&buf);
--
2.23.0
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 09/19] qemu: monitor: Add 'granularity' parameter for block-dirty-bitmap-add
Signed-off-by: Peter Krempa
---
src/qemu/qemu_backup.c | 4 ++--
src/qemu/qemu_checkpoint.c | 2 +-
src/qemu/qemu_monitor.c | 6 --
src/qemu/qemu_monitor.h | 3 ++-
src/qemu/qemu_monitor_json.c | 4 +++-
src/qemu/qemu_monitor_json.h | 3 ++-
tests/qemumonitorjsontest.c | 2 +-
7 files changed, 15 insertions(+), 9 deletions(-)
diff --git a/src/qemu/qemu_backup.c b/src/qemu/qemu_backup.c
index c9709dc29a..5c9747f09d 100644
--- a/src/qemu/qemu_backup.c
+++ b/src/qemu/qemu_backup.c
@@ -201,7 +201,7 @@ qemuBackupDiskPrepareOneBitmaps(struct qemuBackupDiskData
*dd,
dd->domdisk->src->nodeformat,
dd->incrementalBitmap,
false,
-true) < 0)
+true, 0) < 0)
return -1;
if (qemuMonitorTransactionBitmapMerge(actions,
@@ -214,7 +214,7 @@ qemuBackupDiskPrepareOneBitmaps(struct qemuBackupDiskData
*dd,
dd->store->nodeformat,
dd->incrementalBitmap,
false,
-true) < 0)
+true, 0) < 0)
return -1;
if (qemuMonitorTransactionBitmapMerge(actions,
diff --git a/src/qemu/qemu_checkpoint.c b/src/qemu/qemu_checkpoint.c
index 38638c3b1e..97bc97bb8e 100644
--- a/src/qemu/qemu_checkpoint.c
+++ b/src/qemu/qemu_checkpoint.c
@@ -309,7 +309,7 @@ qemuCheckpointAddActions(virDomainObjPtr vm,
if (disk->type != VIR_DOMAIN_CHECKPOINT_TYPE_BITMAP)
continue;
node = qemuDomainDiskNodeFormatLookup(vm, disk->name);
-if (qemuMonitorTransactionBitmapAdd(actions, node, disk->bitmap, true,
false) < 0)
+if (qemuMonitorTransactionBitmapAdd(actions, node, disk->bitmap, true,
false, 0) < 0)
return -1;
/* We only want one active bitmap for a disk along the
diff --git a/src/qemu/qemu_monitor.c b/src/qemu/qemu_monitor.c
index ea3e62dc8e..ccd20b3740 100644
--- a/src/qemu/qemu_monitor.c
+++ b/src/qemu/qemu_monitor.c
@@ -4543,9 +4543,11 @@ qemuMonitorTransactionBitmapAdd(virJSONValuePtr actions,
const char *node,
const char *name,
bool persistent,
-bool disabled)
+bool disabled,
+unsigned long long granularity)
{
-return qemuMonitorJSONTransactionBitmapAdd(actions, node, name,
persistent, disabled);
+return qemuMonitorJSONTransactionBitmapAdd(actions, node, name, persistent,
+ disabled, granularity);
}
diff --git a/src/qemu/qemu_monitor.h b/src/qemu/qemu_monitor.h
index 1c990923d6..3f3b81cddd 100644
--- a/src/qemu/qemu_monitor.h
+++ b/src/qemu/qemu_monitor.h
@@ -1374,7 +1374,8 @@ qemuMonitorTransactionBitmapAdd(virJSONValuePtr actions,
const char *node,
const char *name,
bool persistent,
-bool disabled);
+bool disabled,
+unsigned long long granularity);
int
qemuMonitorTransactionBitmapRemove(virJSONValuePtr actions,
const char *node,
diff --git a/src/qemu/qemu_monitor_json.c b/src/qemu/qemu_monitor_json.c
index 856c2c2778..4e1bcaa30d 100644
--- a/src/qemu/qemu_monitor_json.c
+++ b/src/qemu/qemu_monitor_json.c
@@ -9159,7 +9159,8 @@ qemuMonitorJSONTransactionBitmapAdd(virJSONValuePtr
actions,
const char *node,
const char *name,
bool persistent,
-bool disabled)
+bool disabled,
+unsigned long long granularity)
{
return qemuMonitorJSONTransactionAdd(actions,
"block-dirty-bitmap-add",
@@ -9167,6 +9168,7 @@ qemuMonitorJSONTransactionBitmapAdd(virJSONValuePtr
actions,
"s:name", name,
"b:persistent", persistent,
"b:disabled", disabled,
+ "P:granularity", granularity,
NULL);
}
diff --git a/src/qemu/qemu_monitor_json.h b/src/qemu/qemu_monitor_json.h
index 44926464b9..61f5b0061d 100644
--- a/src/qemu/qemu_monitor_json.h
+++ b/src/qemu/qemu_monitor_json.h
@@ -636,7 +636,8 @@ qemuMonitorJSONTransactionBitmapAdd(virJSONValuePtr actions,
const char *node,
[libvirt] [PATCH 02/19] virsh: Allow extracting 'return' section of QMP command in 'qemu-monitor-command'
Simplify gathering the actual return value from a passed-through QMP
command when using 'qemu-monitor-command' by adding '--return-value'
switch which just extracts the 'return' section and alternatively
reports an error if the section is not present.
This simplifies gathering of some test data where the full reply would
need to be trimmed just for the actual return value.
Signed-off-by: Peter Krempa
---
docs/manpages/virsh.rst | 5 -
tools/virsh-domain.c| 44 -
2 files changed, 39 insertions(+), 10 deletions(-)
diff --git a/docs/manpages/virsh.rst b/docs/manpages/virsh.rst
index 697f83e5b2..7ec620f6ee 100644
--- a/docs/manpages/virsh.rst
+++ b/docs/manpages/virsh.rst
@@ -7434,7 +7434,7 @@ qemu-monitor-command
.. code-block:: shell
- qemu-monitor-command domain { [--hmp] | [--qmp] [--pretty] } command...
+ qemu-monitor-command domain { [--hmp] | [--qmp] [--pretty] [--return-value]
} command...
Send an arbitrary monitor command *command* to domain *domain* through the
qemu monitor. The results of the command will be printed on stdout.
@@ -7451,6 +7451,9 @@ of the QMP command verbatim.
If *--pretty* is given, and the monitor uses QMP, then the output will be
pretty-printed.
+If *--return-value* is given the 'return' key of the QMP response object is
+extracted rather than passing through the full reply from qemu.
+
If *--hmp* is passed, the command is considered to be a human monitor command
and libvirt will automatically convert it into QMP if needed. In that case
the result will also be converted back from QMP.
diff --git a/tools/virsh-domain.c b/tools/virsh-domain.c
index 9447fa2904..a592726042 100644
--- a/tools/virsh-domain.c
+++ b/tools/virsh-domain.c
@@ -9526,6 +9526,10 @@ static const vshCmdOptDef opts_qemu_monitor_command[] = {
.type = VSH_OT_BOOL,
.help = N_("wrap the 'cmd' argument in JSON wrapper for QMP")
},
+{.name = "return-value",
+ .type = VSH_OT_BOOL,
+ .help = N_("extract the value of the 'return' key from the returned
string")
+},
{.name = "cmd",
.type = VSH_OT_ARGV,
.flags = VSH_OFLAG_REQ,
@@ -9540,13 +9544,19 @@ cmdQemuMonitorCommand(vshControl *ctl, const vshCmd
*cmd)
g_autoptr(virshDomain) dom = NULL;
g_autofree char *monitor_cmd = NULL;
g_autofree char *result = NULL;
+g_autoptr(virJSONValue) resultjson = NULL;
unsigned int flags = 0;
const vshCmdOpt *opt = NULL;
virBuffer buf = VIR_BUFFER_INITIALIZER;
bool qmp = vshCommandOptBool(cmd, "qmp");
+bool pretty = vshCommandOptBool(cmd, "pretty");
+bool returnval = vshCommandOptBool(cmd, "return-value");
+virJSONValuePtr formatjson;
+g_autofree char *jsonstr = NULL;
VSH_EXCLUSIVE_OPTIONS("hmp", "pretty");
VSH_EXCLUSIVE_OPTIONS("hmp", "qmp");
+VSH_EXCLUSIVE_OPTIONS("hmp", "return-value");
if (!(dom = virshCommandOptDomain(ctl, cmd, NULL)))
return false;
@@ -9584,17 +9594,33 @@ cmdQemuMonitorCommand(vshControl *ctl, const vshCmd
*cmd)
if (virDomainQemuMonitorCommand(dom, monitor_cmd, &result, flags) < 0)
return false;
-if (vshCommandOptBool(cmd, "pretty")) {
-char *tmp;
-if ((tmp = virJSONStringReformat(result, true))) {
-VIR_FREE(result);
-result = tmp;
-virTrimSpaces(result, NULL);
-} else {
-vshResetLibvirtError();
+if (returnval || pretty) {
+resultjson = virJSONValueFromString(result);
+
+if (returnval && !resultjson) {
+vshError(ctl, "failed to parse JSON returned by qemu");
+return false;
}
}
-vshPrint(ctl, "%s\n", result);
+
+/* print raw non-prettified result */
+if (!resultjson) {
+vshPrint(ctl, "%s\n", result);
+return true;
+}
+
+if (returnval) {
+if (!(formatjson = virJSONValueObjectGet(resultjson, "return"))) {
+vshError(ctl, "'return' member missing");
+return false;
+}
+} else {
+formatjson = resultjson;
+}
+
+jsonstr = virJSONValueToString(formatjson, pretty);
+virTrimSpaces(jsonstr, NULL);
+vshPrint(ctl, "%s", jsonstr);
return true;
}
--
2.23.0
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 00/19] qemu: Add support for backups in combination with snapshots
IMPORTANT: This does NOT add support for merging the bitmaps during block jobs, so manual deletion of snapshots will corrupt the incremental metadata!. Allow combining snapshots and backups. During a snapshot we create a bitmap for any active persistent bitmap to continue tracking the bitmaps properly (as qemu doesn't support creating a dirty bitmap from the allocation map). This also means that it works only for an active VM since there also isn't a way to do it via qemu-img. Thus a workaround is to start a VM as paused if this is required for offline VMs. We also now can merge bitmaps accross the backing chains. You can fetch the changes at: git fetch https://gitlab.com/pipo.sk/libvirt.git backup-snapshots Peter Krempa (19): virsh: Add QMP command wrapping for 'qemu-monitor-command' virsh: Allow extracting 'return' section of QMP command in 'qemu-monitor-command' qemu: monitor: Extract data about dirty-bimaps in qemuMonitorBlockGetNamedNodeData qemu: monitor: Extract internals of qemuMonitorJSONBlockGetNamedNodeData tests: qemublock: Add test for bitmap detection tests: qemublocktest: Add a syntetic test case for bitmap detection qemu: Check for explicit failure of qemuBlockSnapshotAddBlockdev qemu: snapshot: Fold formatting of snapshot transaction into prepare func qemu: monitor: Add 'granularity' parameter for block-dirty-bitmap-add qemu: snapshot: Propagate active bitmaps through external snapshots tests: qemublock: Add test case for detecting bitmaps as we create snapshots qemu: backup: Return 'def' instead of 'obj' from qemuBackupBeginCollectIncrementalCheckpoints qemu: backup: Extract calculations of bitmaps to merge for incremental backup qemu: backup: Propagate bitmap metadata into qemuBackupDiskPrepareOneBitmapsChain qemu: backup: Export qemuBackupDiskPrepareOneBitmapsChain for tests tests: qemublock: Add testing of bitmap merging for incremental backups qemu: block: Introduce qemuBlockNamedNodeDataGetBitmapByName qemu: backup: Merge bitmaps accross the backing chain tests: qemublock: Add tests for cross-snapshot incremental backups docs/manpages/virsh.rst | 27 +- src/qemu/qemu_backup.c| 145 ++- src/qemu/qemu_backup.h| 7 + src/qemu/qemu_block.c | 32 + src/qemu/qemu_block.h | 5 + src/qemu/qemu_checkpoint.c| 2 +- src/qemu/qemu_driver.c| 84 +- src/qemu/qemu_monitor.c | 6 +- src/qemu/qemu_monitor.h | 18 +- src/qemu/qemu_monitor_json.c | 96 +- src/qemu/qemu_monitor_json.h | 6 +- tests/qemublocktest.c | 241 + .../backupmerge/basic-deep-out.json | 22 + .../backupmerge/basic-flat-out.json | 6 + .../backupmerge/basic-intermediate-out.json | 10 + .../backupmerge/snapshot-deep-out.json| 38 + .../backupmerge/snapshot-flat-out.json| 6 + .../snapshot-intermediate-out.json| 14 + tests/qemublocktestdata/bitmap/basic.json | 117 +++ tests/qemublocktestdata/bitmap/basic.out | 6 + tests/qemublocktestdata/bitmap/snapshots.json | 836 ++ tests/qemublocktestdata/bitmap/snapshots.out | 14 + tests/qemublocktestdata/bitmap/synthetic.json | 118 +++ tests/qemublocktestdata/bitmap/synthetic.out | 6 + tests/qemumonitorjsontest.c | 2 +- tools/virsh-domain.c | 76 +- 26 files changed, 1845 insertions(+), 95 deletions(-) create mode 100644 tests/qemublocktestdata/backupmerge/basic-deep-out.json create mode 100644 tests/qemublocktestdata/backupmerge/basic-flat-out.json create mode 100644 tests/qemublocktestdata/backupmerge/basic-intermediate-out.json create mode 100644 tests/qemublocktestdata/backupmerge/snapshot-deep-out.json create mode 100644 tests/qemublocktestdata/backupmerge/snapshot-flat-out.json create mode 100644 tests/qemublocktestdata/backupmerge/snapshot-intermediate-out.json create mode 100644 tests/qemublocktestdata/bitmap/basic.json create mode 100644 tests/qemublocktestdata/bitmap/basic.out create mode 100644 tests/qemublocktestdata/bitmap/snapshots.json create mode 100644 tests/qemublocktestdata/bitmap/snapshots.out create mode 100644 tests/qemublocktestdata/bitmap/synthetic.json create mode 100644 tests/qemublocktestdata/bitmap/synthetic.out -- 2.23.0 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [jenkins-ci PATCH 1/5] guests: Add mapping for augeas-lenses
On Thu, 2019-12-12 at 16:49 +, Jim Fehlig wrote: > On 12/12/19 9:19 AM, Andrea Bolognani wrote: > > On Thu, 2019-12-12 at 15:57 +, Jim Fehlig wrote: > > > If the difference is unwanted we could remove the yast2_basis pattern from > > > autoinst.xml. > > > > What would we lose by doing so? In general we install only the very > > basics through the OS' native installer and then add everything we > > need on top using lcitool, so it certainly sounds like it could make > > sense to drop it... > > It's an installation and config management tool. Obviously we don't need it > for > installation, and I don't think we'll be using it for management within the > VM. Yeah, dropping it sounds like a good idea then. > I can try removing it and send a patch if there are no ill effects, but it > will > have to wait until next week when I have some free time. Take your time... Someone else might even beat you to it ;) -- Andrea Bolognani / Red Hat / Virtualization -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [jenkins-ci PATCH 1/5] guests: Add mapping for augeas-lenses
On 12/12/19 9:19 AM, Andrea Bolognani wrote: > On Thu, 2019-12-12 at 15:57 +, Jim Fehlig wrote: >> On 12/12/19 8:07 AM, Andrea Bolognani wrote: >>> On Thu, 2019-12-12 at 15:43 +0100, Fabiano Fidêncio wrote: On Thu, Dec 12, 2019 at 3:31 PM Andrea Bolognani wrote: > + augeas-lenses: > +default: augeas > +deb: augeas-lenses > +OpenSUSE: augeas-lenses Hmm. I guess it only hits the container, am I right? Because in the VM it's installed as part of the base system. >>> >>> It looks like it's needed by yast: >>> >>> $ sudo zypper remove augeas-lenses >>> Loading repository data... >>> Warning: No repositories defined. Operating only with the installed >>> resolvables. Nothing can be installed. >>> Reading installed packages... >>> Resolving package dependencies... >>> >>> The following 15 packages are going to be REMOVED: >>> augeas-lenses patterns-yast-yast2_basis yast2-bootloader yast2-country >>> yast2-installation yast2-ldap yast2-mail yast2-network >>> yast2-online-update >>> yast2-online-update-frontend yast2-packager yast2-storage-ng >>> yast2-tune >>> yast2-update yast2-users >>> >>> The following pattern is going to be REMOVED: >>> yast2_basis >>> >>> 15 packages to remove. >>> After the operation, 7.5 MiB will be freed. >>> Continue? [y/n/v/...? shows all options] (y): >>> >>> The container is minimal, and doesn't include yast at all: >>> >>> $ rpm -qa | grep yast >>> $ >>> >>> So that would explain the difference. >> >> If the difference is unwanted we could remove the yast2_basis pattern from >> autoinst.xml. > > What would we lose by doing so? In general we install only the very > basics through the OS' native installer and then add everything we > need on top using lcitool, so it certainly sounds like it could make > sense to drop it... It's an installation and config management tool. Obviously we don't need it for installation, and I don't think we'll be using it for management within the VM. I can try removing it and send a patch if there are no ill effects, but it will have to wait until next week when I have some free time. Regards, Jim -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [jenkins-ci PATCH v2 4/6] guests: pull in deps for gtk-vnc project
On Thu, 2019-12-12 at 15:12 +, Daniel P. Berrangé wrote: > +++ b/guests/host_vars/libvirt-fedora-rawhide/main.yml > @@ -1,5 +1,8 @@ > --- > projects: > + - gtk-vnc > + - gtk-vnc+mingw32 > + - gtk-vnc+mingw64 This hunk belongs to guests/host_vars/libvirt-fedora-30/main.yml Everything else looks reasonable, but I want to actually test it before ACKing. -- Andrea Bolognani / Red Hat / Virtualization -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [jenkins-ci PATCH v2 3/6] projects: add gtk-vnc project
On Thu, 2019-12-12 at 15:12 +, Daniel P. Berrangé wrote:
> Signed-off-by: Daniel P. Berrangé
> ---
> guests/playbooks/build/jobs/defaults.yml| 2 ++
> .../build/projects/gtk-vnc+mingw32.yml | 12
> .../build/projects/gtk-vnc+mingw64.yml | 12
> guests/playbooks/build/projects/gtk-vnc.yml | 14 ++
> jenkins/jobs/defaults.yaml | 2 ++
> jenkins/projects/gtk-vnc+mingw32.yaml | 12
> jenkins/projects/gtk-vnc+mingw64.yaml | 12
> jenkins/projects/gtk-vnc.yaml | 17 +
> 8 files changed, 83 insertions(+)
> create mode 100644 guests/playbooks/build/projects/gtk-vnc+mingw32.yml
> create mode 100644 guests/playbooks/build/projects/gtk-vnc+mingw64.yml
> create mode 100644 guests/playbooks/build/projects/gtk-vnc.yml
> create mode 100644 jenkins/projects/gtk-vnc+mingw32.yaml
> create mode 100644 jenkins/projects/gtk-vnc+mingw64.yaml
> create mode 100644 jenkins/projects/gtk-vnc.yaml
It is too soon to introduce build rules for gtk-vnc: first you have
to get the guests/ directory to a point where you can do
./lcitool update all gtk-vnc
and only then you can add build rules.
Basically this patch and the next one need to be swapped.
> +++ b/guests/playbooks/build/projects/gtk-vnc.yml
> @@ -0,0 +1,14 @@
> +---
> +- set_fact:
> +name: gtk-vnc
> +machines: '{{ all_machines }}'
> +archive_format: gz
> +git_url: '{{ git_urls["gtk-vnc"][git_remote] }}'
> +
> +- include: '{{ playbook_base }}/jobs/prepare.yml'
> +- include: '{{ playbook_base }}/jobs/autotools-build-job.yml'
> +- include: '{{ playbook_base }}/jobs/autotools-syntax-check-job.yml'
> +- include: '{{ playbook_base }}/jobs/autotools-check-job.yml'
> +- include: '{{ playbook_base }}/jobs/autotools-rpm-job.yml'
> + vars:
> +machines: '{{ rpm_machines }}'
gtk-vnc is using Meson now, so you need to update this file and
all the other ones to use the corresponding templates instead of
the autotools ones.
--
Andrea Bolognani / Red Hat / Virtualization
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [jenkins-ci PATCH v2 1/6] mappings: add libgcrypt
On Thu, 2019-12-12 at 15:12 +, Daniel P. Berrangé wrote: > +++ b/guests/vars/mappings.yml > + mingw32-libgcrypt: > +FedoraRawhide: mingw32-libgcrypt s/Rawhide// > + mingw64-libgcrypt: > +FedoraRawhide: mingw64-libgcrypt Same here. We're actually using Fedora 30 for MinGW builds at the moment :) With that fixed, Reviewed-by: Andrea Bolognani -- Andrea Bolognani / Red Hat / Virtualization -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [jenkins-ci PATCH 1/5] guests: Add mapping for augeas-lenses
On Thu, 2019-12-12 at 15:57 +, Jim Fehlig wrote: > On 12/12/19 8:07 AM, Andrea Bolognani wrote: > > On Thu, 2019-12-12 at 15:43 +0100, Fabiano Fidêncio wrote: > > > On Thu, Dec 12, 2019 at 3:31 PM Andrea Bolognani > > > wrote: > > > > + augeas-lenses: > > > > +default: augeas > > > > +deb: augeas-lenses > > > > +OpenSUSE: augeas-lenses > > > > > > Hmm. I guess it only hits the container, am I right? Because in the VM > > > it's installed as part of the base system. > > > > It looks like it's needed by yast: > > > >$ sudo zypper remove augeas-lenses > >Loading repository data... > >Warning: No repositories defined. Operating only with the installed > > resolvables. Nothing can be installed. > >Reading installed packages... > >Resolving package dependencies... > > > >The following 15 packages are going to be REMOVED: > > augeas-lenses patterns-yast-yast2_basis yast2-bootloader yast2-country > > yast2-installation yast2-ldap yast2-mail yast2-network > > yast2-online-update > > yast2-online-update-frontend yast2-packager yast2-storage-ng yast2-tune > > yast2-update yast2-users > > > >The following pattern is going to be REMOVED: > > yast2_basis > > > >15 packages to remove. > >After the operation, 7.5 MiB will be freed. > >Continue? [y/n/v/...? shows all options] (y): > > > > The container is minimal, and doesn't include yast at all: > > > >$ rpm -qa | grep yast > >$ > > > > So that would explain the difference. > > If the difference is unwanted we could remove the yast2_basis pattern from > autoinst.xml. What would we lose by doing so? In general we install only the very basics through the OS' native installer and then add everything we need on top using lcitool, so it certainly sounds like it could make sense to drop it... -- Andrea Bolognani / Red Hat / Virtualization -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [jenkins-ci PATCH 1/5] guests: Add mapping for augeas-lenses
On Thu, 2019-12-12 at 16:42 +0100, Pino Toscano wrote: > OTOH, in the mapping file I see: > >augeas: > default: augeas > deb: augeas-tools > > This means that most probably there are different packages installed > depending on the distro: > - on Debian-based distros you get the command line tools > - on some distros (mostly RPM-based) you get only the shared library > - on some other distros (FreeBSD, and Linux distros not handled yet) > you may get either everything, or the command line tools I guess a better name for the mapping would indeed be 'augtool', because that's what we're trying to install. And it works as intended, even on CentOS and Fedora despite what you wrote above. > OTOH², looking at the spec [1] makes me sad, as nothing really > depends on augeas-lenses, neither the shared library package nor the > tools package :-( This is totally nonsense, you install augeas in > openSUSE and it is broken by default if you have recommends disabled... YaST2 apparently requires augeas-lenses[1], so while I agree that the current situation is probably broken the issue will not actually not show up in most openSUSE installations. > So my suggestion is to limit the manual lenses installation only on > openSUSE, as any other distro is already doing a sane job in this case. We usually don't do that. Even though we would expect that installing augtool would bring in the lenses as well, there's really nothing inherently wrong with installing the lenses explicitly: different distros have different granularity, so it's quite common that we end up resolving multiple mappings to the same concrete package, and in fact we have code in place to deal with just that situation. tl;dr openSUSE might want to fix their dependencies, but as far as I'm concerned the current situation in libvirt-jenkins-ci is perfectly fine and I'm in no hurry to change it. [1] https://www.redhat.com/archives/libvir-list/2019-December/msg00858.html -- Andrea Bolognani / Red Hat / Virtualization -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [jenkins-ci PATCH 0/5] lcitool: Make Dockerfile generation work on openSUSE
On 12/12/19 8:31 AM, Andrea Bolognani wrote: > On Thu, 2019-12-12 at 15:45 +0100, Fabiano Fidêncio wrote: >> On Thu, Dec 12, 2019 at 3:31 PM Andrea Bolognani wrote: >>> Andrea Bolognani (5): >>>guests: Add mapping for augeas-lenses >>>guests: Install augeas-lenses for libvirt >>>lcitool: Remove redundant update step for openSUSE >>>guests: Add base container image for openSUSE Leap 15.1 >>>lcitool: Make Dockerfile generation work on openSUSE >> >> There's one suggestion in the last patch. Please, take a look at that >> and feel free to adopt the suggestion or not. >> >> Reviewed-by: Fabiano Fidêncio > > Thanks, I've pushed the series now. I'm confident Jim will point out > any silly mistake I might have made because of my lack of experience > with openSUSE :) It looks good to me! Thanks for taking care of one of the tasks on my list :-). Regards, Jim -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [jenkins-ci PATCH 1/5] guests: Add mapping for augeas-lenses
On 12/12/19 8:07 AM, Andrea Bolognani wrote: > On Thu, 2019-12-12 at 15:43 +0100, Fabiano Fidêncio wrote: >> On Thu, Dec 12, 2019 at 3:31 PM Andrea Bolognani wrote: >>> + augeas-lenses: >>> +default: augeas >>> +deb: augeas-lenses >>> +OpenSUSE: augeas-lenses >> >> Hmm. I guess it only hits the container, am I right? Because in the VM >> it's installed as part of the base system. > > It looks like it's needed by yast: > >$ sudo zypper remove augeas-lenses >Loading repository data... >Warning: No repositories defined. Operating only with the installed > resolvables. Nothing can be installed. >Reading installed packages... >Resolving package dependencies... > >The following 15 packages are going to be REMOVED: > augeas-lenses patterns-yast-yast2_basis yast2-bootloader yast2-country > yast2-installation yast2-ldap yast2-mail yast2-network > yast2-online-update > yast2-online-update-frontend yast2-packager yast2-storage-ng yast2-tune > yast2-update yast2-users > >The following pattern is going to be REMOVED: > yast2_basis > >15 packages to remove. >After the operation, 7.5 MiB will be freed. >Continue? [y/n/v/...? shows all options] (y): > > The container is minimal, and doesn't include yast at all: > >$ rpm -qa | grep yast >$ > > So that would explain the difference. If the difference is unwanted we could remove the yast2_basis pattern from autoinst.xml. Regards, Jim -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [jenkins-ci PATCH] lcitool: Only install EPEL on CentOS
This fixes a bug introduced by commit 0a7993d3ed30 which caused our machinery to try to install the epel-release on all targets, including the likes of Debian and even FreeBSD. Signed-off-by: Andrea Bolognani --- Pushed under the build breaker rule. guests/playbooks/update/tasks/base.yml | 2 ++ 1 file changed, 2 insertions(+) diff --git a/guests/playbooks/update/tasks/base.yml b/guests/playbooks/update/tasks/base.yml index 8128975..f721e39 100644 --- a/guests/playbooks/update/tasks/base.yml +++ b/guests/playbooks/update/tasks/base.yml @@ -21,6 +21,8 @@ package: name: epel-release state: latest + when: +- os_name == 'CentOS' - name: Create OpenVZ key template: -- 2.23.0 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [jenkins-ci PATCH 1/5] guests: Add mapping for augeas-lenses
On Thursday, 12 December 2019 16:26:26 CET Andrea Bolognani wrote: > On Thu, 2019-12-12 at 16:05 +0100, Pino Toscano wrote: > > On Thursday, 12 December 2019 15:31:04 CET Andrea Bolognani wrote: > > > + augeas-lenses: > > > +default: augeas > > > +deb: augeas-lenses > > > +OpenSUSE: augeas-lenses > > > > Why is this needed? The lenses must be already a dependency of the > > augeas shared library, or at least of the tools. > > Not on openSUSE, apparently: > > $ docker run --rm -it opensuse/leap:15.1 > edb149088975:/ # zypper install augeas > Building repository 'Non-OSS Repository' cache .. > Building repository 'Main Repository' cache . > Building repository 'Main Update Repository' cache .. > Building repository 'Update Repository (Non-Oss)' cache . > Loading repository data... > Reading installed packages... > Resolving package dependencies... > > The following NEW package is going to be installed: > augeas > > 1 new package to install. > Overall download size: 130.1 KiB. Already cached: 0 B. After the operation, > additional 205.1 KiB will be used. > Continue? [y/n/v/...? shows all options] (y): y > Retrieving package augeas-1.10.1-lp151.2.3.x86_64 (1/1), 130.1 KiB > (205.1 KiB unpacked) > Retrieving: augeas-1.10.1-lp151.2.3.x86_64.rpm .. > > Checking for file conflicts: > (1/1) Installing: augeas-1.10.1-lp151.2.3.x86_64 > edb149088975:/ # rpm -qa | grep augeas-lenses > edb149088975:/ # ls /usr/share/augeas/lenses > ls: cannot access '/usr/share/augeas/lenses': No such file or directory > edb149088975:/ # Sigh... OTOH, in the mapping file I see: augeas: default: augeas deb: augeas-tools This means that most probably there are different packages installed depending on the distro: - on Debian-based distros you get the command line tools - on some distros (mostly RPM-based) you get only the shared library - on some other distros (FreeBSD, and Linux distros not handled yet) you may get either everything, or the command line tools OTOH², looking at the spec [1] makes me sad, as nothing really depends on augeas-lenses, neither the shared library package nor the tools package :-( This is totally nonsense, you install augeas in openSUSE and it is broken by default if you have recommends disabled... So my suggestion is to limit the manual lenses installation only on openSUSE, as any other distro is already doing a sane job in this case. [1] https://build.opensuse.org/package/view_file/openSUSE:Factory/augeas/augeas.spec -- Pino Toscano signature.asc Description: This is a digitally signed message part. -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [jenkins-ci PATCH 0/5] lcitool: Make Dockerfile generation work on openSUSE
On Thu, 2019-12-12 at 15:45 +0100, Fabiano Fidêncio wrote: > On Thu, Dec 12, 2019 at 3:31 PM Andrea Bolognani wrote: > > Andrea Bolognani (5): > > guests: Add mapping for augeas-lenses > > guests: Install augeas-lenses for libvirt > > lcitool: Remove redundant update step for openSUSE > > guests: Add base container image for openSUSE Leap 15.1 > > lcitool: Make Dockerfile generation work on openSUSE > > There's one suggestion in the last patch. Please, take a look at that > and feel free to adopt the suggestion or not. > > Reviewed-by: Fabiano Fidêncio Thanks, I've pushed the series now. I'm confident Jim will point out any silly mistake I might have made because of my lack of experience with openSUSE :) -- Andrea Bolognani / Red Hat / Virtualization -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [jenkins-ci PATCH 1/5] guests: Add mapping for augeas-lenses
On Thu, 2019-12-12 at 16:05 +0100, Pino Toscano wrote: > On Thursday, 12 December 2019 15:31:04 CET Andrea Bolognani wrote: > > + augeas-lenses: > > +default: augeas > > +deb: augeas-lenses > > +OpenSUSE: augeas-lenses > > Why is this needed? The lenses must be already a dependency of the > augeas shared library, or at least of the tools. Not on openSUSE, apparently: $ docker run --rm -it opensuse/leap:15.1 edb149088975:/ # zypper install augeas Building repository 'Non-OSS Repository' cache .. Building repository 'Main Repository' cache . Building repository 'Main Update Repository' cache .. Building repository 'Update Repository (Non-Oss)' cache . Loading repository data... Reading installed packages... Resolving package dependencies... The following NEW package is going to be installed: augeas 1 new package to install. Overall download size: 130.1 KiB. Already cached: 0 B. After the operation, additional 205.1 KiB will be used. Continue? [y/n/v/...? shows all options] (y): y Retrieving package augeas-1.10.1-lp151.2.3.x86_64 (1/1), 130.1 KiB (205.1 KiB unpacked) Retrieving: augeas-1.10.1-lp151.2.3.x86_64.rpm .. Checking for file conflicts: (1/1) Installing: augeas-1.10.1-lp151.2.3.x86_64 edb149088975:/ # rpm -qa | grep augeas-lenses edb149088975:/ # ls /usr/share/augeas/lenses ls: cannot access '/usr/share/augeas/lenses': No such file or directory edb149088975:/ # ¯\_(ツ)_/¯ -- Andrea Bolognani / Red Hat / Virtualization -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [jenkins-ci PATCH v2 3/6] projects: add gtk-vnc project
Signed-off-by: Daniel P. Berrangé
---
guests/playbooks/build/jobs/defaults.yml| 2 ++
.../build/projects/gtk-vnc+mingw32.yml | 12
.../build/projects/gtk-vnc+mingw64.yml | 12
guests/playbooks/build/projects/gtk-vnc.yml | 14 ++
jenkins/jobs/defaults.yaml | 2 ++
jenkins/projects/gtk-vnc+mingw32.yaml | 12
jenkins/projects/gtk-vnc+mingw64.yaml | 12
jenkins/projects/gtk-vnc.yaml | 17 +
8 files changed, 83 insertions(+)
create mode 100644 guests/playbooks/build/projects/gtk-vnc+mingw32.yml
create mode 100644 guests/playbooks/build/projects/gtk-vnc+mingw64.yml
create mode 100644 guests/playbooks/build/projects/gtk-vnc.yml
create mode 100644 jenkins/projects/gtk-vnc+mingw32.yaml
create mode 100644 jenkins/projects/gtk-vnc+mingw64.yaml
create mode 100644 jenkins/projects/gtk-vnc.yaml
diff --git a/guests/playbooks/build/jobs/defaults.yml
b/guests/playbooks/build/jobs/defaults.yml
index 5e4ec03..5cbc440 100644
--- a/guests/playbooks/build/jobs/defaults.yml
+++ b/guests/playbooks/build/jobs/defaults.yml
@@ -46,6 +46,8 @@ mingw64_local_env: |
mingw64_autogen_args: --host=x86_64-w64-mingw32
mingw64_meson_args: --cross-file=/usr/share/mingw/toolchain-mingw64.meson
git_urls:
+ gtk-vnc:
+default: https://gitlab.gnome.org/GNOME/gtk-vnc.git
libosinfo:
default: https://gitlab.com/libosinfo/libosinfo.git
libvirt-cim:
diff --git a/guests/playbooks/build/projects/gtk-vnc+mingw32.yml
b/guests/playbooks/build/projects/gtk-vnc+mingw32.yml
new file mode 100644
index 000..0f7c8f3
--- /dev/null
+++ b/guests/playbooks/build/projects/gtk-vnc+mingw32.yml
@@ -0,0 +1,12 @@
+---
+- set_fact:
+name: gtk-vnc+mingw32
+machines: '{{ mingw_machines }}'
+archive_format: gz
+git_url: '{{ git_urls["gtk-vnc"][git_remote] }}'
+
+- include: '{{ playbook_base }}/jobs/prepare.yml'
+- include: '{{ playbook_base }}/jobs/autotools-build-job.yml'
+ vars:
+local_env: '{{ mingw32_local_env }}'
+autogen_args: '{{ mingw32_autogen_args }}'
diff --git a/guests/playbooks/build/projects/gtk-vnc+mingw64.yml
b/guests/playbooks/build/projects/gtk-vnc+mingw64.yml
new file mode 100644
index 000..1cbc75c
--- /dev/null
+++ b/guests/playbooks/build/projects/gtk-vnc+mingw64.yml
@@ -0,0 +1,12 @@
+---
+- set_fact:
+name: gtk-vnc+mingw64
+machines: '{{ mingw_machines }}'
+archive_format: gz
+git_url: '{{ git_urls["gtk-vnc"][git_remote] }}'
+
+- include: '{{ playbook_base }}/jobs/prepare.yml'
+- include: '{{ playbook_base }}/jobs/autotools-build-job.yml'
+ vars:
+local_env: '{{ mingw64_local_env }}'
+autogen_args: '{{ mingw64_autogen_args }}'
diff --git a/guests/playbooks/build/projects/gtk-vnc.yml
b/guests/playbooks/build/projects/gtk-vnc.yml
new file mode 100644
index 000..6fa4942
--- /dev/null
+++ b/guests/playbooks/build/projects/gtk-vnc.yml
@@ -0,0 +1,14 @@
+---
+- set_fact:
+name: gtk-vnc
+machines: '{{ all_machines }}'
+archive_format: gz
+git_url: '{{ git_urls["gtk-vnc"][git_remote] }}'
+
+- include: '{{ playbook_base }}/jobs/prepare.yml'
+- include: '{{ playbook_base }}/jobs/autotools-build-job.yml'
+- include: '{{ playbook_base }}/jobs/autotools-syntax-check-job.yml'
+- include: '{{ playbook_base }}/jobs/autotools-check-job.yml'
+- include: '{{ playbook_base }}/jobs/autotools-rpm-job.yml'
+ vars:
+machines: '{{ rpm_machines }}'
diff --git a/jenkins/jobs/defaults.yaml b/jenkins/jobs/defaults.yaml
index 676ecbf..2d9ffef 100644
--- a/jenkins/jobs/defaults.yaml
+++ b/jenkins/jobs/defaults.yaml
@@ -42,6 +42,8 @@
mingw64_autogen_args: --host=x86_64-w64-mingw32
mingw64_meson_args: --cross-file=/usr/share/mingw/toolchain-mingw64.meson
git_urls:
+ gtk-vnc:
+default: https://gitlab.gnome.org/GNOME/gtk-vnc.git
libosinfo:
default: https://gitlab.com/libosinfo/libosinfo.git
libvirt-cim:
diff --git a/jenkins/projects/gtk-vnc+mingw32.yaml
b/jenkins/projects/gtk-vnc+mingw32.yaml
new file mode 100644
index 000..22b1ad2
--- /dev/null
+++ b/jenkins/projects/gtk-vnc+mingw32.yaml
@@ -0,0 +1,12 @@
+---
+- project:
+name: gtk-vnc+mingw32
+machines: '{mingw_machines}'
+title: GTK-VNC MinGW (32-bit)
+archive_format: gz
+git_url: '{git_urls[gtk-vnc][default]}'
+jobs:
+ - autotools-build-job:
+ parent_jobs:
+ local_env: '{mingw32_local_env}'
+ autogen_args: '{mingw32_autogen_args}'
diff --git a/jenkins/projects/gtk-vnc+mingw64.yaml
b/jenkins/projects/gtk-vnc+mingw64.yaml
new file mode 100644
index 000..06cd94f
--- /dev/null
+++ b/jenkins/projects/gtk-vnc+mingw64.yaml
@@ -0,0 +1,12 @@
+---
+- project:
+name: gtk-vnc+mingw64
+machines: '{mingw_machines}'
+title: GTK-VNC MinGW (64-bit)
+archive_format: gz
+git_url: '{git_urls[gtk-vnc][default]}'
+jobs:
+ - autotools-b
[libvirt] [jenkins-ci PATCH v2 4/6] guests: pull in deps for gtk-vnc project
Signed-off-by: Daniel P. Berrangé --- guests/host_vars/libvirt-centos-7/main.yml| 1 + guests/host_vars/libvirt-centos-8/main.yml| 1 + guests/host_vars/libvirt-debian-10/main.yml | 1 + guests/host_vars/libvirt-debian-9/main.yml| 1 + guests/host_vars/libvirt-debian-sid/main.yml | 1 + guests/host_vars/libvirt-fedora-30/main.yml | 1 + guests/host_vars/libvirt-fedora-31/main.yml | 1 + guests/host_vars/libvirt-fedora-rawhide/main.yml | 3 +++ guests/host_vars/libvirt-freebsd-11/main.yml | 1 + guests/host_vars/libvirt-freebsd-12/main.yml | 1 + guests/host_vars/libvirt-freebsd-current/main.yml | 1 + guests/host_vars/libvirt-ubuntu-1604/main.yml | 1 + guests/host_vars/libvirt-ubuntu-1804/main.yml | 1 + guests/vars/projects/gtk-vnc+mingw32.yml | 6 ++ guests/vars/projects/gtk-vnc+mingw64.yml | 6 ++ guests/vars/projects/gtk-vnc.yml | 11 +++ 16 files changed, 38 insertions(+) create mode 100644 guests/vars/projects/gtk-vnc+mingw32.yml create mode 100644 guests/vars/projects/gtk-vnc+mingw64.yml create mode 100644 guests/vars/projects/gtk-vnc.yml diff --git a/guests/host_vars/libvirt-centos-7/main.yml b/guests/host_vars/libvirt-centos-7/main.yml index e0579ad..becd297 100644 --- a/guests/host_vars/libvirt-centos-7/main.yml +++ b/guests/host_vars/libvirt-centos-7/main.yml @@ -1,5 +1,6 @@ --- projects: + - gtk-vnc - libosinfo - libvirt - libvirt-cim diff --git a/guests/host_vars/libvirt-centos-8/main.yml b/guests/host_vars/libvirt-centos-8/main.yml index aae2313..cb49e9c 100644 --- a/guests/host_vars/libvirt-centos-8/main.yml +++ b/guests/host_vars/libvirt-centos-8/main.yml @@ -1,5 +1,6 @@ --- projects: + - gtk-vnc - libosinfo - libvirt - libvirt-dbus diff --git a/guests/host_vars/libvirt-debian-10/main.yml b/guests/host_vars/libvirt-debian-10/main.yml index 633f421..c5d0047 100644 --- a/guests/host_vars/libvirt-debian-10/main.yml +++ b/guests/host_vars/libvirt-debian-10/main.yml @@ -1,5 +1,6 @@ --- projects: + - gtk-vnc - libosinfo - libvirt - libvirt-dbus diff --git a/guests/host_vars/libvirt-debian-9/main.yml b/guests/host_vars/libvirt-debian-9/main.yml index 6b685a4..3addcd4 100644 --- a/guests/host_vars/libvirt-debian-9/main.yml +++ b/guests/host_vars/libvirt-debian-9/main.yml @@ -1,5 +1,6 @@ --- projects: + - gtk-vnc - libosinfo - libvirt - libvirt-dbus diff --git a/guests/host_vars/libvirt-debian-sid/main.yml b/guests/host_vars/libvirt-debian-sid/main.yml index 3808383..a60dd1b 100644 --- a/guests/host_vars/libvirt-debian-sid/main.yml +++ b/guests/host_vars/libvirt-debian-sid/main.yml @@ -1,5 +1,6 @@ --- projects: + - gtk-vnc - libosinfo - libvirt - libvirt-dbus diff --git a/guests/host_vars/libvirt-fedora-30/main.yml b/guests/host_vars/libvirt-fedora-30/main.yml index e395f32..0e9466e 100644 --- a/guests/host_vars/libvirt-fedora-30/main.yml +++ b/guests/host_vars/libvirt-fedora-30/main.yml @@ -1,5 +1,6 @@ --- projects: + - gtk-vnc - libosinfo - libosinfo+mingw32 - libosinfo+mingw64 diff --git a/guests/host_vars/libvirt-fedora-31/main.yml b/guests/host_vars/libvirt-fedora-31/main.yml index 5d9a1b5..e605873 100644 --- a/guests/host_vars/libvirt-fedora-31/main.yml +++ b/guests/host_vars/libvirt-fedora-31/main.yml @@ -1,5 +1,6 @@ --- projects: + - gtk-vnc - libosinfo - libvirt - libvirt-cim diff --git a/guests/host_vars/libvirt-fedora-rawhide/main.yml b/guests/host_vars/libvirt-fedora-rawhide/main.yml index fc39363..0567099 100644 --- a/guests/host_vars/libvirt-fedora-rawhide/main.yml +++ b/guests/host_vars/libvirt-fedora-rawhide/main.yml @@ -1,5 +1,8 @@ --- projects: + - gtk-vnc + - gtk-vnc+mingw32 + - gtk-vnc+mingw64 - libosinfo - libvirt - libvirt-cim diff --git a/guests/host_vars/libvirt-freebsd-11/main.yml b/guests/host_vars/libvirt-freebsd-11/main.yml index e9f6d03..2de64ef 100644 --- a/guests/host_vars/libvirt-freebsd-11/main.yml +++ b/guests/host_vars/libvirt-freebsd-11/main.yml @@ -1,5 +1,6 @@ --- projects: + - gtk-vnc - libosinfo - libvirt - libvirt-dbus diff --git a/guests/host_vars/libvirt-freebsd-12/main.yml b/guests/host_vars/libvirt-freebsd-12/main.yml index ba3ba62..2e3b935 100644 --- a/guests/host_vars/libvirt-freebsd-12/main.yml +++ b/guests/host_vars/libvirt-freebsd-12/main.yml @@ -1,5 +1,6 @@ --- projects: + - gtk-vnc - libosinfo - libvirt - libvirt-dbus diff --git a/guests/host_vars/libvirt-freebsd-current/main.yml b/guests/host_vars/libvirt-freebsd-current/main.yml index 74e1856..9b63d30 100644 --- a/guests/host_vars/libvirt-freebsd-current/main.yml +++ b/guests/host_vars/libvirt-freebsd-current/main.yml @@ -1,5 +1,6 @@ --- projects: + - gtk-vnc - libosinfo - libvirt - libvirt-dbus diff --git a/guests/host_vars/libvirt-ubuntu-1604/main.yml b/guests/host_vars/libvirt-ubuntu-1604/main.yml index 4f803a5..27
[libvirt] [jenkins-ci PATCH v2 6/6] mappings: remove gtk-vnc2
Reviewed-by: Andrea Bolognani Signed-off-by: Daniel P. Berrangé --- guests/vars/mappings.yml | 13 - 1 file changed, 13 deletions(-) diff --git a/guests/vars/mappings.yml b/guests/vars/mappings.yml index 40f06ab..3c74ee6 100644 --- a/guests/vars/mappings.yml +++ b/guests/vars/mappings.yml @@ -234,13 +234,6 @@ mappings: default: gtk-update-icon-cache Ubuntu1604: libgtk2.0-bin - gtk-vnc2: -deb: libgtk-vnc-2.0-dev -pkg: gtk-vnc -rpm: gtk-vnc2-devel -CentOS8: -cross-policy-deb: foreign - hal: FreeBSD: hal @@ -518,9 +511,6 @@ mappings: mingw32-gtk3: Fedora: mingw32-gtk3 - mingw32-gtk-vnc2: -Fedora: mingw32-gtk-vnc2 - mingw32-json-glib: Fedora: mingw32-json-glib @@ -602,9 +592,6 @@ mappings: mingw64-gtk3: Fedora: mingw64-gtk3 - mingw64-gtk-vnc2: -Fedora: mingw64-gtk-vnc2 - mingw64-json-glib: Fedora: mingw64-json-glib -- 2.23.0 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [jenkins-ci PATCH v2 2/6] mappings: add PulseAudio
Reviewed-by: Andrea Bolognani Signed-off-by: Daniel P. Berrangé --- guests/vars/mappings.yml | 5 + 1 file changed, 5 insertions(+) diff --git a/guests/vars/mappings.yml b/guests/vars/mappings.yml index 786a609..40f06ab 100644 --- a/guests/vars/mappings.yml +++ b/guests/vars/mappings.yml @@ -828,6 +828,11 @@ mappings: default: polkit deb: policykit-1 + pulseaudio: +deb: libpulse-dev +rpm: pulseaudio-libs-devel +cross-policy-deb: foreign + python3-docutils: default: python3-docutils CentOS7: python36-docutils -- 2.23.0 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [jenkins-ci PATCH v2 5/6] projects: make virt-viewer depend on gtk-vnc jobs
Reviewed-by: Andrea Bolognani
Signed-off-by: Daniel P. Berrangé
---
guests/playbooks/build/jobs/defaults.yml | 1 +
guests/vars/projects/virt-viewer+mingw32.yml | 1 -
guests/vars/projects/virt-viewer+mingw64.yml | 1 -
guests/vars/projects/virt-viewer.yml | 1 -
jenkins/jobs/defaults.yaml | 1 +
jenkins/projects/virt-viewer+mingw32.yaml| 4 +++-
jenkins/projects/virt-viewer+mingw64.yaml| 4 +++-
jenkins/projects/virt-viewer.yaml| 4 +++-
8 files changed, 11 insertions(+), 6 deletions(-)
diff --git a/guests/playbooks/build/jobs/defaults.yml
b/guests/playbooks/build/jobs/defaults.yml
index 5cbc440..65dee78 100644
--- a/guests/playbooks/build/jobs/defaults.yml
+++ b/guests/playbooks/build/jobs/defaults.yml
@@ -32,6 +32,7 @@ strip_buildrequires: |
sed -i -e 's/BuildRequires: *libvirt.*//' *.spec*
sed -i -e 's/BuildRequires: *osinfo-db.*//' *.spec*
sed -i -e 's/BuildRequires: *perl(Sys::Virt).*//' *.spec*
+ sed -i -e 's/BuildRequires: *pkgconfig(gtk-vnc-.*).*//' *.spec*
sed -i -e 's/BuildRequires: *pkgconfig(libvirt.*).*//' *.spec*
mingw32_local_env: |
export VIRT_PREFIX="$VIRT_PREFIX/i686-w64-mingw32/sys-root/mingw"
diff --git a/guests/vars/projects/virt-viewer+mingw32.yml
b/guests/vars/projects/virt-viewer+mingw32.yml
index 2b914c3..608d722 100644
--- a/guests/vars/projects/virt-viewer+mingw32.yml
+++ b/guests/vars/projects/virt-viewer+mingw32.yml
@@ -6,7 +6,6 @@ packages:
- mingw32-gstreamer1-plugins-bad-free
- mingw32-gstreamer1-plugins-good
- mingw32-gtk3
- - mingw32-gtk-vnc2
- mingw32-libgovirt
- mingw32-libusbx
- mingw32-rest
diff --git a/guests/vars/projects/virt-viewer+mingw64.yml
b/guests/vars/projects/virt-viewer+mingw64.yml
index 6b42a7f..3aa5893 100644
--- a/guests/vars/projects/virt-viewer+mingw64.yml
+++ b/guests/vars/projects/virt-viewer+mingw64.yml
@@ -6,7 +6,6 @@ packages:
- mingw64-gstreamer1-plugins-bad-free
- mingw64-gstreamer1-plugins-good
- mingw64-gtk3
- - mingw64-gtk-vnc2
- mingw64-libgovirt
- mingw64-libusbx
- mingw64-rest
diff --git a/guests/vars/projects/virt-viewer.yml
b/guests/vars/projects/virt-viewer.yml
index c03d50b..fe754cb 100644
--- a/guests/vars/projects/virt-viewer.yml
+++ b/guests/vars/projects/virt-viewer.yml
@@ -1,7 +1,6 @@
---
packages:
- glib2
- - gtk-vnc2
- gtk3
- libgovirt
- libxml2
diff --git a/jenkins/jobs/defaults.yaml b/jenkins/jobs/defaults.yaml
index 2d9ffef..ec429ed 100644
--- a/jenkins/jobs/defaults.yaml
+++ b/jenkins/jobs/defaults.yaml
@@ -28,6 +28,7 @@
sed -i -e 's/BuildRequires: *libvirt.*//' *.spec*
sed -i -e 's/BuildRequires: *osinfo-db.*//' *.spec*
sed -i -e 's/BuildRequires: *perl(Sys::Virt).*//' *.spec*
+ sed -i -e 's/BuildRequires: *pkgconfig(gtk-vnc-.*).*//' *.spec*
sed -i -e 's/BuildRequires: *pkgconfig(libvirt.*).*//' *.spec*
mingw32_local_env: |
export VIRT_PREFIX="$VIRT_PREFIX/i686-w64-mingw32/sys-root/mingw"
diff --git a/jenkins/projects/virt-viewer+mingw32.yaml
b/jenkins/projects/virt-viewer+mingw32.yaml
index c9c74ea..7d4738a 100644
--- a/jenkins/projects/virt-viewer+mingw32.yaml
+++ b/jenkins/projects/virt-viewer+mingw32.yaml
@@ -7,6 +7,8 @@
git_url: '{git_urls[virt-viewer][default]}'
jobs:
- autotools-build-job:
- parent_jobs: 'libvirt-glib+mingw32-build'
+ parent_jobs:
+- 'libvirt-glib+mingw32-build'
+- 'gtk-vnc+mingw32-build'
local_env: '{mingw32_local_env}'
autogen_args: '{mingw32_autogen_args}'
diff --git a/jenkins/projects/virt-viewer+mingw64.yaml
b/jenkins/projects/virt-viewer+mingw64.yaml
index c3b570f..dfbd70d 100644
--- a/jenkins/projects/virt-viewer+mingw64.yaml
+++ b/jenkins/projects/virt-viewer+mingw64.yaml
@@ -7,6 +7,8 @@
git_url: '{git_urls[virt-viewer][default]}'
jobs:
- autotools-build-job:
- parent_jobs: 'libvirt-glib+mingw64-build'
+ parent_jobs:
+- 'libvirt-glib+mingw64-build'
+- 'gtk-vnc+mingw64-build'
local_env: '{mingw64_local_env}'
autogen_args: '{mingw64_autogen_args}'
diff --git a/jenkins/projects/virt-viewer.yaml
b/jenkins/projects/virt-viewer.yaml
index 6469b9f..123f95e 100644
--- a/jenkins/projects/virt-viewer.yaml
+++ b/jenkins/projects/virt-viewer.yaml
@@ -7,7 +7,9 @@
git_url: '{git_urls[virt-viewer][default]}'
jobs:
- autotools-build-job:
- parent_jobs: 'libvirt-glib-build'
+ parent_jobs:
+- 'libvirt-glib-build'
+- 'gtk-vnc-build'
- autotools-syntax-check-job:
parent_jobs: 'virt-viewer-build'
- autotools-check-job:
--
2.23.0
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [jenkins-ci PATCH v2 1/6] mappings: add libgcrypt
Reviewed-by: Andrea Bolognani Signed-off-by: Daniel P. Berrangé --- guests/vars/mappings.yml | 12 1 file changed, 12 insertions(+) diff --git a/guests/vars/mappings.yml b/guests/vars/mappings.yml index 69d95f3..786a609 100644 --- a/guests/vars/mappings.yml +++ b/guests/vars/mappings.yml @@ -331,6 +331,12 @@ mappings: OpenSUSE: dbus-1-devel cross-policy-deb: foreign + libgcrypt: +deb: libgcrypt20-dev +pkg: libgcrypt +rpm: libgcrypt-devel +cross-policy-deb: foreign + libgovirt: rpm: libgovirt-devel CentOS8: @@ -521,6 +527,9 @@ mappings: mingw32-libarchive: Fedora: mingw32-libarchive + mingw32-libgcrypt: +FedoraRawhide: mingw32-libgcrypt + mingw32-libgovirt: Fedora: mingw32-libgovirt @@ -602,6 +611,9 @@ mappings: mingw64-libarchive: Fedora: mingw64-libarchive + mingw64-libgcrypt: +FedoraRawhide: mingw64-libgcrypt + mingw64-libgovirt: Fedora: mingw64-libgovirt -- 2.23.0 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [jenkins-ci PATCH v2 0/6] Add support for gtk-vnc builds
Support gtk-vnc and make virt-viewer depend on it I sent this months ago & forgot to push it. Re-sending just in case something has invalidated any patches. I've at least added the newer distros Daniel P. Berrangé (6): mappings: add libgcrypt mappings: add PulseAudio projects: add gtk-vnc project guests: pull in deps for gtk-vnc project projects: make virt-viewer depend on gtk-vnc jobs mappings: remove gtk-vnc2 guests/host_vars/libvirt-centos-7/main.yml| 1 + guests/host_vars/libvirt-centos-8/main.yml| 1 + guests/host_vars/libvirt-debian-10/main.yml | 1 + guests/host_vars/libvirt-debian-9/main.yml| 1 + guests/host_vars/libvirt-debian-sid/main.yml | 1 + guests/host_vars/libvirt-fedora-30/main.yml | 1 + guests/host_vars/libvirt-fedora-31/main.yml | 1 + .../host_vars/libvirt-fedora-rawhide/main.yml | 3 ++ guests/host_vars/libvirt-freebsd-11/main.yml | 1 + guests/host_vars/libvirt-freebsd-12/main.yml | 1 + .../libvirt-freebsd-current/main.yml | 1 + guests/host_vars/libvirt-ubuntu-1604/main.yml | 1 + guests/host_vars/libvirt-ubuntu-1804/main.yml | 1 + guests/playbooks/build/jobs/defaults.yml | 3 ++ .../build/projects/gtk-vnc+mingw32.yml| 12 .../build/projects/gtk-vnc+mingw64.yml| 12 guests/playbooks/build/projects/gtk-vnc.yml | 14 + guests/vars/mappings.yml | 30 +++ guests/vars/projects/gtk-vnc+mingw32.yml | 6 guests/vars/projects/gtk-vnc+mingw64.yml | 6 guests/vars/projects/gtk-vnc.yml | 11 +++ guests/vars/projects/virt-viewer+mingw32.yml | 1 - guests/vars/projects/virt-viewer+mingw64.yml | 1 - guests/vars/projects/virt-viewer.yml | 1 - jenkins/jobs/defaults.yaml| 3 ++ jenkins/projects/gtk-vnc+mingw32.yaml | 12 jenkins/projects/gtk-vnc+mingw64.yaml | 12 jenkins/projects/gtk-vnc.yaml | 17 +++ jenkins/projects/virt-viewer+mingw32.yaml | 4 ++- jenkins/projects/virt-viewer+mingw64.yaml | 4 ++- jenkins/projects/virt-viewer.yaml | 4 ++- 31 files changed, 149 insertions(+), 19 deletions(-) create mode 100644 guests/playbooks/build/projects/gtk-vnc+mingw32.yml create mode 100644 guests/playbooks/build/projects/gtk-vnc+mingw64.yml create mode 100644 guests/playbooks/build/projects/gtk-vnc.yml create mode 100644 guests/vars/projects/gtk-vnc+mingw32.yml create mode 100644 guests/vars/projects/gtk-vnc+mingw64.yml create mode 100644 guests/vars/projects/gtk-vnc.yml create mode 100644 jenkins/projects/gtk-vnc+mingw32.yaml create mode 100644 jenkins/projects/gtk-vnc+mingw64.yaml create mode 100644 jenkins/projects/gtk-vnc.yaml -- 2.23.0 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [jenkins-ci PATCH 1/5] guests: Add mapping for augeas-lenses
On Thu, 2019-12-12 at 15:43 +0100, Fabiano Fidêncio wrote: > On Thu, Dec 12, 2019 at 3:31 PM Andrea Bolognani wrote: > > + augeas-lenses: > > +default: augeas > > +deb: augeas-lenses > > +OpenSUSE: augeas-lenses > > Hmm. I guess it only hits the container, am I right? Because in the VM > it's installed as part of the base system. It looks like it's needed by yast: $ sudo zypper remove augeas-lenses Loading repository data... Warning: No repositories defined. Operating only with the installed resolvables. Nothing can be installed. Reading installed packages... Resolving package dependencies... The following 15 packages are going to be REMOVED: augeas-lenses patterns-yast-yast2_basis yast2-bootloader yast2-country yast2-installation yast2-ldap yast2-mail yast2-network yast2-online-update yast2-online-update-frontend yast2-packager yast2-storage-ng yast2-tune yast2-update yast2-users The following pattern is going to be REMOVED: yast2_basis 15 packages to remove. After the operation, 7.5 MiB will be freed. Continue? [y/n/v/...? shows all options] (y): The container is minimal, and doesn't include yast at all: $ rpm -qa | grep yast $ So that would explain the difference. -- Andrea Bolognani / Red Hat / Virtualization -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [jenkins-ci PATCH 1/5] guests: Add mapping for augeas-lenses
On Thursday, 12 December 2019 15:31:04 CET Andrea Bolognani wrote: > Some distributions package augeas and the default lenses separately, > so we need a mapping for the latter. > > Signed-off-by: Andrea Bolognani > --- > guests/vars/mappings.yml | 5 + > 1 file changed, 5 insertions(+) > > diff --git a/guests/vars/mappings.yml b/guests/vars/mappings.yml > index d32ecf3..02bedab 100644 > --- a/guests/vars/mappings.yml > +++ b/guests/vars/mappings.yml > @@ -77,6 +77,11 @@ mappings: > default: augeas > deb: augeas-tools > > + augeas-lenses: > +default: augeas > +deb: augeas-lenses > +OpenSUSE: augeas-lenses Why is this needed? The lenses must be already a dependency of the augeas shared library, or at least of the tools. -- Pino Toscano signature.asc Description: This is a digitally signed message part. -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [jenkins-ci PATCH 5/5] lcitool: Make Dockerfile generation work on openSUSE
On Thu, 2019-12-12 at 15:39 +0100, Fabiano Fidêncio wrote:
> > +# openSUSE doesn't seem to have a convenient way to remove all
> > +# unnecessary packages, but CentOS and Fedora do
> > +if os_name == "OpenSUSE":
> > +commands.extend([
> > +"{package_manager} clean --all",
> > +])
> > +else:
> > +commands.extend([
> > +"{package_manager} autoremove -y",
> > +"{package_manager} clean all -y",
> > +])
> > +
>
> IMHO, it'd be easier to follow / cleaner if we do:
> if os_name != "OpenSUSE:"
> commands.extend([
> "{package_manager} autoremove -y",
>])
>
> And then, later on ...
> commands.extend([
> "{package_manager} clean all -y",
> ])
Nope, that wouldn't work: the subcommand is
clean --all
on openSUSE and
clean all -y
everywhere else, soo there's no part that's common.
--
Andrea Bolognani / Red Hat / Virtualization
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] libvirt API/design questions
On Wed, Dec 11, 2019 at 09:45:07AM -0500, Cole Robinson wrote: * Splitting domain_conf up. There's 30k lines in domain_conf.c and 3500 in domain_conf.h, it would be nice to unwind it a bit. Getting some sign off on this ahead of time is critical IMO so pieces can be posted and committed quickly because they will obviously go out of date fast. Oh yes, so fast I did not even keep the branch for this failed attempt: https://www.redhat.com/archives/libvir-list/2019-July/msg01257.html Jano My thoughts: - domain_def.[ch]: DomainDefXXX and enum definitions. - probably New and Free functions too - domain_parse.[ch]: XML parsing - domain_format.[ch]: XML formatting - domain_validate.[ch]: validate and postparse handling - domain_util.[ch]: everything else, or keep it in domain_conf? domain_def should be easy but no idea how intertwined the rest are. If the domain_validate naming is acceptable for both validate and postparse functions, we could use that naming for qemu_domain.c split too. Also maybe it's worth considering if there's some way to sensibly split the conf/ directory. We could add a top level domain/ directory, but that's kinda ambiguously named as we already have network/ + storage/ + secret/ etc that have different meanings. Maybe sub dirs like conf/domain/ ? I'm curious if anyone has strong feelings either way. There's not really a clear place to dump shared DomainDef code at the moment, like possible domain_cgroup for sharing cgroup handling across lxc + qemu Thanks, Cole -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list signature.asc Description: PGP signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 1/8] qemu: don't take agent and monitor job for shutdown
On 12/5/19 5:08 PM, Jonathon Jongsma wrote:
We have to assume that the guest agent may be malicious so we don't want
to allow any agent queries to block any other libvirt API. By holding
a monitor job while we're querying the agent, we open ourselves up to a
DoS. So split the function into separate parts: one that does the agent
shutdown and one that does the monitor shutdown. Each part holds only a
job of the appropriate type.
Signed-off-by: Jonathon Jongsma
---
src/qemu/qemu_driver.c | 116 +
1 file changed, 72 insertions(+), 44 deletions(-)
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index 1911073f3e..92efde72dd 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -1929,6 +1929,72 @@ static int qemuDomainResume(virDomainPtr dom)
return ret;
}
+static int qemuDomainShutdownFlagsAgent(virQEMUDriverPtr driver,
+virDomainObjPtr vm,
+bool isReboot,
+bool reportError)
Nitpick, new functions should be written as
static int
qemuDomainBlahBlah()
{
}
Michal
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 6/8] qemu: don't hold monitor job for GetGuestInfo()
On 12/5/19 5:08 PM, Jonathon Jongsma wrote: We have to assume that the guest agent may be malicious so we don't want to allow any agent queries to block any other libvirt API. By holding a monitor job while we're querying the agent, we open ourselves up to a DoS. This function issues several agent commands, but does not issue any monitor commands. Therefore, we can drop the monitor job and only hold an agent job. Signed-off-by: Jonathon Jongsma --- src/qemu/qemu_driver.c | 7 +++ 1 file changed, 3 insertions(+), 4 deletions(-) Same reasoning as to the previous one. Michal -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 0/8] Don't hold both monitor and agent jobs at the same time
On 12/5/19 5:08 PM, Jonathon Jongsma wrote: We have to assume that the guest agent may be malicious, so we don't want to allow any agent queries to block any other libvirt API. By holding a monitor job and an agent job while we're querying the agent, any other threads will be blocked from using the monitor while the agent is unresponsive. Because libvirt waits forever for an agent response, this makes us vulnerable to a denial of service from a malicious (or simply buggy) guest agent. This series of patches attempts to remove any cases where we were holding both jobs at the same time, removes a convenience function which allows us to grab both jobs at once, and updates documentation regarding this issue. Jonathon Jongsma (8): qemu: don't take agent and monitor job for shutdown qemu: don't hold a monitor and agent job for reboot qemu: don't hold both jobs for suspend qemu: don't hold monitor and agent job when setting time qemu: don't hold monitor job for fsinfo qemu: don't hold monitor job for GetGuestInfo() qemu: remove use of qemuDomainObjBeginJobWithAgent() qemu: remove qemuDomainObjBegin/EndJobWithAgent() src/qemu/THREADS.txt | 58 +- src/qemu/qemu_domain.c | 56 +- src/qemu/qemu_domain.h | 7 - src/qemu/qemu_driver.c | 405 + 4 files changed, 258 insertions(+), 268 deletions(-) ACK to all but 5/8 and 6/8. Also, I'm pushing patches 1-4 and 7. I'd push 8/8 also but we can't remove the function while it's still use :-D Michal -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 2/8] qemu: don't hold a monitor and agent job for reboot
On 12/5/19 5:08 PM, Jonathon Jongsma wrote: We have to assume that the guest agent may be malicious so we don't want to allow any agent queries to block any other libvirt API. By holding a monitor job while we're querying the agent, we open ourselves up to a DoS. Split the function so that we only hold the appropriate type of job while rebooting. Signed-off-by: Jonathon Jongsma --- src/qemu/qemu_driver.c | 111 + 1 file changed, 68 insertions(+), 43 deletions(-) diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 92efde72dd..edd36f4a89 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -2097,56 +2157,21 @@ qemuDomainReboot(virDomainPtr dom, unsigned int flags) +if (ret < 0 && agentForced) +goto cleanup; Ooops, misaligned. Michal -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 3/8] qemu: don't hold both jobs for suspend
On 12/5/19 5:08 PM, Jonathon Jongsma wrote:
We have to assume that the guest agent may be malicious so we don't want
to allow any agent queries to block any other libvirt API. By holding a
monitor job while we're querying the agent, we open ourselves up to a
DoS.
So split the function up a bit to only hold the monitor job while
querying qemu for whether the domain supports suspend. Then acquire only
an agent job while issuing the agent suspend command.
Signed-off-by: Jonathon Jongsma
---
src/qemu/qemu_driver.c | 93 ++
1 file changed, 58 insertions(+), 35 deletions(-)
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index edd36f4a89..e39ee2acc9 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -19713,6 +19713,58 @@ qemuDomainProbeQMPCurrentMachine(virQEMUDriverPtr
driver,
}
+/* returns -1 on error, or if query is not supported, 0 if query was successful */
+static int
+qemuDomainQueryWakeupSuspendSupport(virQEMUDriverPtr driver,
+virDomainObjPtr vm,
+bool *wakeupSupported)
+{
+int ret = -1;
+qemuDomainObjPrivatePtr priv = vm->privateData;
Usually, I put @ret last as it's shorter line compared to @priv.
+
+if (!virQEMUCapsGet(priv->qemuCaps, QEMU_CAPS_QUERY_CURRENT_MACHINE))
+return ret;
s/ret/-1/
+
+if (qemuDomainObjBeginJob(driver, vm, QEMU_JOB_MODIFY) < 0)
+return ret;
+
Michal
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 5/8] qemu: don't hold monitor job for fsinfo
On 12/5/19 5:08 PM, Jonathon Jongsma wrote: We have to assume that the guest agent may be malicious so we don't want to allow any agent queries to block any other libvirt API. By holding a monitor job while we're querying the agent, we open ourselves up to a DoS. This function does not issue any monitor commands, so we can drop the monitor job and only hold an agent job. While this is true, the reason I've added BeginJobWithAgent() call is that qemuAgentGetFSInfo() works with vm->def which may change beneath our hands since we wouldn't be taking a vm job. This is potentially dangerous and may lead to a crash (as @vm is unlocked and not guarded by any job). What we need to do is to create a copy of vm->def and pass that to qemuAgentGetFSInfo(). However, creating a copy of domain definition is very expensive - esp. when the agent monitor function needs only a list of disk targets. So we might construct the list beforehand and pass that to the function. Then taking only agent job is going to be okay. Signed-off-by: Jonathon Jongsma --- src/qemu/qemu_driver.c | 7 +++ 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 10fad8d75d..e1a91c5049 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -21623,9 +21623,8 @@ qemuDomainGetFSInfo(virDomainPtr dom, if (virDomainGetFSInfoEnsureACL(dom->conn, vm->def) < 0) goto cleanup; -if (qemuDomainObjBeginJobWithAgent(driver, vm, - QEMU_JOB_QUERY, - QEMU_AGENT_JOB_QUERY) < 0) +if (qemuDomainObjBeginAgentJob(driver, vm, + QEMU_AGENT_JOB_QUERY) < 0) goto cleanup; if (virDomainObjCheckActive(vm) < 0) @@ -21639,7 +21638,7 @@ qemuDomainGetFSInfo(virDomainPtr dom, qemuDomainObjExitAgent(vm, agent); endjob: -qemuDomainObjEndJobWithAgent(driver, vm); +qemuDomainObjEndAgentJob(vm); cleanup: virDomainObjEndAPI(&vm); I won't push this one, sorry. Michal -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [jenkins-ci PATCH 0/5] lcitool: Make Dockerfile generation work on openSUSE
On Thu, Dec 12, 2019 at 3:31 PM Andrea Bolognani wrote: > > With this series applied, running > > ./lcitool dockerfile libvirt-opensuse-151 libvirt > > results in a Dockerfile that can be successfully used to build a > container image, and that image in turn can be successfully used > to build libvirt in. > > Andrea Bolognani (5): > guests: Add mapping for augeas-lenses > guests: Install augeas-lenses for libvirt > lcitool: Remove redundant update step for openSUSE > guests: Add base container image for openSUSE Leap 15.1 > lcitool: Make Dockerfile generation work on openSUSE > > guests/host_vars/libvirt-opensuse-151/docker.yml | 2 ++ > guests/lcitool | 14 -- > guests/playbooks/update/tasks/base.yml | 7 --- > guests/vars/mappings.yml | 5 + > guests/vars/projects/libvirt.yml | 1 + > 5 files changed, 20 insertions(+), 9 deletions(-) > create mode 100644 guests/host_vars/libvirt-opensuse-151/docker.yml > > -- > 2.23.0 > There's one suggestion in the last patch. Please, take a look at that and feel free to adopt the suggestion or not. Reviewed-by: Fabiano Fidêncio -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [jenkins-ci PATCH 1/5] guests: Add mapping for augeas-lenses
On Thu, Dec 12, 2019 at 3:31 PM Andrea Bolognani wrote: > > Some distributions package augeas and the default lenses separately, > so we need a mapping for the latter. > > Signed-off-by: Andrea Bolognani > --- > guests/vars/mappings.yml | 5 + > 1 file changed, 5 insertions(+) > > diff --git a/guests/vars/mappings.yml b/guests/vars/mappings.yml > index d32ecf3..02bedab 100644 > --- a/guests/vars/mappings.yml > +++ b/guests/vars/mappings.yml > @@ -77,6 +77,11 @@ mappings: > default: augeas > deb: augeas-tools > > + augeas-lenses: > +default: augeas > +deb: augeas-lenses > +OpenSUSE: augeas-lenses Hmm. I guess it only hits the container, am I right? Because in the VM it's installed as part of the base system. -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [jenkins-ci PATCH 5/5] lcitool: Make Dockerfile generation work on openSUSE
[snip]
>
> +# openSUSE doesn't seem to have a convenient way to remove all
> +# unnecessary packages, but CentOS and Fedora do
> +if os_name == "OpenSUSE":
> +commands.extend([
> +"{package_manager} clean --all",
> +])
> +else:
> +commands.extend([
> +"{package_manager} autoremove -y",
> +"{package_manager} clean all -y",
> +])
> +
IMHO, it'd be easier to follow / cleaner if we do:
if os_name != "OpenSUSE:"
commands.extend([
"{package_manager} autoremove -y",
])
And then, later on ...
commands.extend([
"{package_manager} clean all -y",
])
[snip]
Best Regards,
--
Fabiano Fidêncio
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [jenkins-ci PATCH 5/5] lcitool: Make Dockerfile generation work on openSUSE
We can't use the same command sequence as for CentOS and Fedora.
Signed-off-by: Andrea Bolognani
---
guests/lcitool | 14 --
1 file changed, 12 insertions(+), 2 deletions(-)
diff --git a/guests/lcitool b/guests/lcitool
index 9958508..d862282 100755
--- a/guests/lcitool
+++ b/guests/lcitool
@@ -767,10 +767,20 @@ class Application:
commands.extend([
"{package_manager} update -y",
"{package_manager} install -y {pkgs}",
-"{package_manager} autoremove -y",
-"{package_manager} clean all -y",
])
+# openSUSE doesn't seem to have a convenient way to remove all
+# unnecessary packages, but CentOS and Fedora do
+if os_name == "OpenSUSE":
+commands.extend([
+"{package_manager} clean --all",
+])
+else:
+commands.extend([
+"{package_manager} autoremove -y",
+"{package_manager} clean all -y",
+])
+
script = "\nRUN " + (" && \\\n".join(commands)) + "\n"
sys.stdout.write(script.format(**varmap))
--
2.23.0
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [jenkins-ci PATCH 3/5] lcitool: Remove redundant update step for openSUSE
We already use the package module to update all RPM-based distros
just a few lines above.
Signed-off-by: Andrea Bolognani
---
guests/playbooks/update/tasks/base.yml | 7 ---
1 file changed, 7 deletions(-)
diff --git a/guests/playbooks/update/tasks/base.yml
b/guests/playbooks/update/tasks/base.yml
index b3ab738..188ef0a 100644
--- a/guests/playbooks/update/tasks/base.yml
+++ b/guests/playbooks/update/tasks/base.yml
@@ -72,13 +72,6 @@
when:
- package_format == 'pkg'
-- name: Update installed packages
- command: '{{ package_manager }} update -y -l --force-resolution
--no-recommends'
- args:
-warn: no
- when:
-- os_name == 'OpenSUSE'
-
- name: Clean up packages after update
shell: '{{ package_manager }} clean packages -y && {{ package_manager }}
autoremove -y'
args:
--
2.23.0
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [jenkins-ci PATCH 1/5] guests: Add mapping for augeas-lenses
Some distributions package augeas and the default lenses separately, so we need a mapping for the latter. Signed-off-by: Andrea Bolognani --- guests/vars/mappings.yml | 5 + 1 file changed, 5 insertions(+) diff --git a/guests/vars/mappings.yml b/guests/vars/mappings.yml index d32ecf3..02bedab 100644 --- a/guests/vars/mappings.yml +++ b/guests/vars/mappings.yml @@ -77,6 +77,11 @@ mappings: default: augeas deb: augeas-tools + augeas-lenses: +default: augeas +deb: augeas-lenses +OpenSUSE: augeas-lenses + autoconf: default: autoconf -- 2.23.0 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [jenkins-ci PATCH 0/5] lcitool: Make Dockerfile generation work on openSUSE
With this series applied, running ./lcitool dockerfile libvirt-opensuse-151 libvirt results in a Dockerfile that can be successfully used to build a container image, and that image in turn can be successfully used to build libvirt in. Andrea Bolognani (5): guests: Add mapping for augeas-lenses guests: Install augeas-lenses for libvirt lcitool: Remove redundant update step for openSUSE guests: Add base container image for openSUSE Leap 15.1 lcitool: Make Dockerfile generation work on openSUSE guests/host_vars/libvirt-opensuse-151/docker.yml | 2 ++ guests/lcitool | 14 -- guests/playbooks/update/tasks/base.yml | 7 --- guests/vars/mappings.yml | 5 + guests/vars/projects/libvirt.yml | 1 + 5 files changed, 20 insertions(+), 9 deletions(-) create mode 100644 guests/host_vars/libvirt-opensuse-151/docker.yml -- 2.23.0 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [jenkins-ci PATCH 4/5] guests: Add base container image for openSUSE Leap 15.1
Signed-off-by: Andrea Bolognani --- guests/host_vars/libvirt-opensuse-151/docker.yml | 2 ++ 1 file changed, 2 insertions(+) create mode 100644 guests/host_vars/libvirt-opensuse-151/docker.yml diff --git a/guests/host_vars/libvirt-opensuse-151/docker.yml b/guests/host_vars/libvirt-opensuse-151/docker.yml new file mode 100644 index 000..6ba7adb --- /dev/null +++ b/guests/host_vars/libvirt-opensuse-151/docker.yml @@ -0,0 +1,2 @@ +--- +docker_base: opensuse/leap:15.1 -- 2.23.0 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [jenkins-ci PATCH 2/5] guests: Install augeas-lenses for libvirt
libvirt's 'make check' requires the default lenses to be available. Signed-off-by: Andrea Bolognani --- guests/vars/projects/libvirt.yml | 1 + 1 file changed, 1 insertion(+) diff --git a/guests/vars/projects/libvirt.yml b/guests/vars/projects/libvirt.yml index 780a5aa..b35e44a 100644 --- a/guests/vars/projects/libvirt.yml +++ b/guests/vars/projects/libvirt.yml @@ -2,6 +2,7 @@ packages: - apparmor - augeas + - augeas-lenses - avahi - cyrus-sasl - device-mapper -- 2.23.0 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [RFC PATCH 0/9] add virtiofs support (virtio-fs epopee)
Hi Ján,
Could you add no_posix_lock and no_flock option for virtiofsd as well?
The sample patch is as follows:
---
docs/schemas/domaincommon.rng | 10 ++
src/conf/domain_conf.c| 10 ++
src/conf/domain_conf.h| 2 ++
src/qemu/qemu_extdevice.c | 10 ++
4 files changed, 32 insertions(+)
diff --git a/docs/schemas/domaincommon.rng b/docs/schemas/domaincommon.rng
index f6479c9..6dd8e2a 100644
--- a/docs/schemas/domaincommon.rng
+++ b/docs/schemas/domaincommon.rng
@@ -2523,6 +2523,16 @@
+
+
+
+
+
+
+
+
+
+
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index 2694e4b..8223910 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -11055,6 +11055,10 @@ virDomainFSDefParseXML(virDomainXMLOptionPtr xmlopt,
target = virXMLPropString(cur, "dir");
} else if (virXMLNodeNameEqual(cur, "readonly")) {
def->readonly = true;
+} else if (virXMLNodeNameEqual(cur, "no_posix_lock")) {
+def->no_posix_lock = true;
+} else if (virXMLNodeNameEqual(cur, "no_flock")) {
+def->no_flock = true;
} else if (virXMLNodeNameEqual(cur, "driver")) {
if (!fsdriver)
fsdriver = virXMLPropString(cur, "type");
@@ -24899,6 +24903,12 @@ virDomainFSDefFormat(virBufferPtr buf,
if (def->readonly)
virBufferAddLit(buf, "\n");
+if (def->no_posix_lock)
+virBufferAddLit(buf, "\n");
+
+if (def->no_flock)
+virBufferAddLit(buf, "\n");
+
if (virDomainDeviceInfoFormat(buf, &def->info, flags) < 0)
goto cleanup;
diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h
index 78f88a0..8a60c8c 100644
--- a/src/conf/domain_conf.h
+++ b/src/conf/domain_conf.h
@@ -812,6 +812,8 @@ struct _virDomainFSDef {
virStorageSourcePtr src;
char *dst;
bool readonly;
+bool no_posix_lock;
+bool no_flock;
virDomainDeviceInfo info;
unsigned long long space_hard_limit; /* in bytes */
unsigned long long space_soft_limit; /* in bytes */
diff --git a/src/qemu/qemu_extdevice.c b/src/qemu/qemu_extdevice.c
index 634a3fb..618a886 100644
--- a/src/qemu/qemu_extdevice.c
+++ b/src/qemu/qemu_extdevice.c
@@ -242,6 +242,16 @@ qemuExtVirtioFSdStart(virQEMUDriverPtr driver,
virCommandAddArgFormat(cmd, "source=%s", fs->src->path);
virCommandAddArg(cmd, "-d");
+if (fs->no_posix_lock) {
+virCommandAddArg(cmd, "-o");
+virCommandAddArg(cmd, "no_posix_lock");
+}
+
+if (fs->no_flock) {
+virCommandAddArg(cmd, "-o");
+virCommandAddArg(cmd, "no_flock");
+}
+
if (qemuExtDeviceLogCommand(driver, vm, cmd, "virtiofsd") < 0)
goto cleanup;
--
2.18.1
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v4 6/7] remote: shrink the critical sections
On Wed, Dec 11, 2019 at 07:48 PM -0500, Cole Robinson wrote: > On 11/14/19 12:44 PM, Marc Hartmayer wrote: >> To free the structs and save the error, it is not necessary to hold >> @priv->lock, >> therefore move these parts after the mutex unlock. >> >> Signed-off-by: Marc Hartmayer >> --- >> src/remote/remote_daemon_dispatch.c | 32 ++--- >> 1 file changed, 16 insertions(+), 16 deletions(-) > > Reviewed-by: Cole Robinson > > Do I understand correctly that 1,3-5 are all independent and can be > pushed separately? If so I will do that tomorrow. I'm doing some > archaeology on patch #2 1, 3, and 5 are all independent. Patch 4 depends on the second patch as remoteDispatchConnectRegisterCloseCallback uses virConnectRegisterCloseCallback. Otherwise we would never do the unref for @client and @program when conn->driver->connectRegisterCloseCallback was NULL. > > - Cole > -- Kind regards / Beste Grüße Marc Hartmayer IBM Deutschland Research & Development GmbH Vorsitzende des Aufsichtsrats: Matthias Hartmann Geschäftsführung: Dirk Wittkopp Sitz der Gesellschaft: Böblingen Registergericht: Amtsgericht Stuttgart, HRB 243294 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] libvirt API/design questions
On Thu, Dec 12, 2019 at 11:13:02 +, Daniel Berrange wrote: > On Wed, Dec 11, 2019 at 09:45:07AM -0500, Cole Robinson wrote: [...] > > * vhost-user-scsi and vhost-user-blk XML schema > > https://www.redhat.com/archives/libvirt-users/2019-October/msg00018.html > > > > Last proposal was using for this, which revisiting it now > > makes more sense than , because it vhost-user-X doesn't seem to > > use qemu's block layer at all. So vhost-user-scsi would be: > > > > > >> path='/path/to/vhost-user-scsi.sock' mode='client'/> > > > > > > vhost-user-blk maybe requires a new type='block' ? Otherwise it's > > unclear how to differentiate between the two. Regardless it would be > > nice to give the original patch author guidance here, they seemed > > motivated to get this working > > This is a really tricky question in general. It is basically saying > whether we consider to refer to the guest visible device > type or the QEMU visible backend type. > > Originally these were always the same thing, but offloading to > vhostuser has blurred the boundaries. I think in non-QEMU hypervisors > where you don't have a split of frontend&backend in the config, you'd > just have disks no matter what. > > With network we've continued to use with vhost-user. > > This makes me biased towards , at least for vhost-user-blk. Alternatively we can make it more clear by properly using the attribute. If qemu is driving the backend we put 'qemu' in it, but other values can apply. This will also make sense e.g. when the qemu storage daemon will become a thing to configure that interface without confusion. -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [jenkins-ci PATCH 2/2] lcitool: Refactor Dockerfile generation
On Thu, Dec 12, 2019 at 3:05 PM Andrea Bolognani wrote:
>
> On Thu, 2019-12-12 at 14:49 +0100, Fabiano Fidêncio wrote:
> > > +commands.extend([
> > > +"{package_manager} update -y",
> > > +"{package_manager} install -y {pkgs}",
> > > +"{package_manager} autoremove -y",
> >
> > This is going to be fun when we enable OpenSUSE support for container
> > generation. `zypper autoremove -y` is not a valid command and we'll
> > have to break this part.
>
> I already have patches for openSUSE locally - I'm actually testing
> them as we speak :)
Oh, me too. :-)
>
> > With this in mind, I'm not sure whether it'd be worth to have a proper
> > mapping of the distros and the commands they support, having Fedora /
> > CentOS as the base.
> >
> > What do you think?
> >
> > Mind, I'm not pushing for the mapping, just pointing it out. :-). I'm
> > fine with an `if os_name != "OpenSUSE": ...`
>
> Yeah, even with all this it's still kinda yucky, but much better
> than what we have now. Adding another level of mapping doesn't seem
> worth it to me, but feel free to experiment with it and send patches
> if you end up with something that improves on the status quo.
I'm fine with the status quo.
>
> --
> Andrea Bolognani / Red Hat / Virtualization
>
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [jenkins-ci PATCH 0/2] lcitool: Refactor Dockerfile generation
On Thu, Dec 12, 2019 at 2:23 PM Andrea Bolognani wrote: > > Andrea Bolognani (2): > lcitool: Perform system update after enabling repos > lcitool: Refactor Dockerfile generation > > guests/lcitool | 66 +- > 1 file changed, 33 insertions(+), 33 deletions(-) > > -- > 2.23.0 > > -- > libvir-list mailing list > libvir-list@redhat.com > https://www.redhat.com/mailman/listinfo/libvir-list > Reviewed-by: Fabiano Fidêncio -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [jenkins-ci PATCH 2/2] lcitool: Refactor Dockerfile generation
On Thu, 2019-12-12 at 14:49 +0100, Fabiano Fidêncio wrote:
> > +commands.extend([
> > +"{package_manager} update -y",
> > +"{package_manager} install -y {pkgs}",
> > +"{package_manager} autoremove -y",
>
> This is going to be fun when we enable OpenSUSE support for container
> generation. `zypper autoremove -y` is not a valid command and we'll
> have to break this part.
I already have patches for openSUSE locally - I'm actually testing
them as we speak :)
> With this in mind, I'm not sure whether it'd be worth to have a proper
> mapping of the distros and the commands they support, having Fedora /
> CentOS as the base.
>
> What do you think?
>
> Mind, I'm not pushing for the mapping, just pointing it out. :-). I'm
> fine with an `if os_name != "OpenSUSE": ...`
Yeah, even with all this it's still kinda yucky, but much better
than what we have now. Adding another level of mapping doesn't seem
worth it to me, but feel free to experiment with it and send patches
if you end up with something that improves on the status quo.
--
Andrea Bolognani / Red Hat / Virtualization
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [jenkins-ci PATCH 2/2] lcitool: Refactor Dockerfile generation
[snip]
> +
> +commands.extend([
> +"{package_manager} update -y",
> +"{package_manager} install -y {pkgs}",
> +"{package_manager} autoremove -y",
This is going to be fun when we enable OpenSUSE support for container
generation. `zypper autoremove -y` is not a valid command and we'll
have to break this part.
With this in mind, I'm not sure whether it'd be worth to have a proper
mapping of the distros and the commands they support, having Fedora /
CentOS as the base.
What do you think?
Mind, I'm not pushing for the mapping, just pointing it out. :-). I'm
fine with an `if os_name != "OpenSUSE": ...`
[snip]
Best Regards,
--
Fabiano Fidêncio
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v4 2/7] virConnectRegisterCloseCallback: Cleanup 'opaque' if there is no connectRegisterCloseCallback
On Wed, Dec 11, 2019 at 08:11 PM -0500, Cole Robinson
wrote:
> On 11/14/19 12:44 PM, Marc Hartmayer wrote:
>> The commit 'close callback: move it to driver' (88f09b75eb99) moved
>> the responsibility for the close callback to the driver. But if the
>> driver doesn't support the connectRegisterCloseCallback API this
>> function does nothing, even no unsupported error report. This behavior
>> may lead to problems, for example memory leaks, as the caller cannot
>> differentiate whether the close callback was 'really' registered or
>> not.
>>
>
>
> Full context:
> v1 subtrhead with jferlan and danpb:
> https://www.redhat.com/archives/libvir-list/2018-March/msg00906.html
> https://www.redhat.com/archives/libvir-list/2018-April/msg00116.html
>
> v2 subthread with john:
> https://www.redhat.com/archives/libvir-list/2018-April/msg02534.html
>
> My first thought is, why not just make this API start raising an error
> if it isn't supported?
>
> But you tried that here:
> https://www.redhat.com/archives/libvir-list/2018-March/msg00405.html
First, thanks for doing all the “history research”.
>
> I'm not really sure I buy the argument that we can't change the
> semantics of the API here. This is the only callback API that seems to
> not raise an explicit error. It's documented to raise an error. And
> there's possible memory leak due the ambiguity.
If we’re doing this so let’s fix the behavior of
'virConnectUnregisterCloseCallback' as well.
>
> Yeah I see that virsh needs to be updated to match. In practice virsh
> shouldn't be a problem: this issue will only hit for local drivers, and
> virsh and client library will be updated together for that case.
>
> In theory if a python app is using this API and not expecting an
> exception, it could cause a regression. But it's also informing them
> that, hey, that connection callback you requested wasn't working in the
> first place. So it's arguably a correctness issue.
>
> So IMO we should be able to adjust this to return a proper error.
>
>
> BUT, if we stick with this direction, then we need to extend the doc
> text here to describe all of this:
>
> * Returns -1 if the driver can support close callback, but registering
> one failed. User must free opaque?
> * Returns 0 if the driver does not support close callback. We will free
> data for you
> * Returns 0 if the driver successfully registered a close callback. When
> that callback is triggered, opaque will be free'd
>
> But that sounds pretty nutty IMO :/
I know…
>
>> Therefore call directly @freecb if the connectRegisterCloseCallback
>> API is not supported by the driver used by the connection and update
>> the documentation.
>>
>> Signed-off-by: Marc Hartmayer
>> ---
>> src/libvirt-host.c | 16 +++-
>> 1 file changed, 11 insertions(+), 5 deletions(-)
>>
>> diff --git a/src/libvirt-host.c b/src/libvirt-host.c
>> index 221a1b7a4353..94383ed449a9 100644
>> --- a/src/libvirt-host.c
>> +++ b/src/libvirt-host.c
>> @@ -1398,7 +1398,7 @@ virConnectIsAlive(virConnectPtr conn)
>> * @conn: pointer to connection object
>> * @cb: callback to invoke upon close
>> * @opaque: user data to pass to @cb
>> - * @freecb: callback to free @opaque
>> + * @freecb: callback to free @opaque when not used anymore
>> *
>> * Registers a callback to be invoked when the connection
>> * is closed. This callback is invoked when there is any
>> @@ -1412,7 +1412,9 @@ virConnectIsAlive(virConnectPtr conn)
>> *
>> * The @freecb must not invoke any other libvirt public
>> * APIs, since it is not called from a re-entrant safe
>> - * context.
>> + * context. Only if virConnectRegisterCloseCallback is
>> + * successful, @freecb will be called, otherwise the
>> + * caller is responsible to free @opaque.
>
> This reads wrong to me. If cb() is successfully registered, then
> freecb() is invoked automatically after cb() is called, right?
Yep, or if the callback is unregistered.
> This
> comment makes it sound like freecb() is invoked immediately when
> virConnectRegisterCloseCallback returns 0
I can reword it.
>
> Hopefully I'm not confusing things more :)
No, I appreciate that you’re looking for it.
>
> - Cole
>
>> *
>> * Returns 0 on success, -1 on error
>> */
>> @@ -1428,9 +1430,13 @@ virConnectRegisterCloseCallback(virConnectPtr conn,
>> virCheckConnectReturn(conn, -1);
>> virCheckNonNullArgGoto(cb, error);
>>
>> -if (conn->driver->connectRegisterCloseCallback &&
>> -conn->driver->connectRegisterCloseCallback(conn, cb, opaque,
>> freecb) < 0)
>> -goto error;
>> +if (conn->driver->connectRegisterCloseCallback) {
>> +if (conn->driver->connectRegisterCloseCallback(conn, cb, opaque,
>> freecb) < 0)
>> +goto error;
>> +} else {
>> +if (freecb)
>> +freecb(opaque);
>> +}
>>
>> return 0;
>>
>>
>
--
Kind regards / Beste Grüße
Marc Hartmayer
IBM Deutschland Research & Development GmbH
Vorsitzende des Aufsichtsrats: Ma
[libvirt] [jenkins-ci PATCH 2/2] lcitool: Refactor Dockerfile generation
The current code is quite a mess, with the same commands being
repeated over and over again with very minor variations based on
necessities that are not spelled out at all. Refactor it and solve
both issues in the process; the output is entirely unchanged.
Signed-off-by: Andrea Bolognani
---
guests/lcitool | 66 +-
1 file changed, 33 insertions(+), 33 deletions(-)
diff --git a/guests/lcitool b/guests/lcitool
index 4c2a04e..9958508 100755
--- a/guests/lcitool
+++ b/guests/lcitool
@@ -740,40 +740,40 @@ class Application:
{package_manager} autoclean -y
""").format(**varmap))
elif package_format == "rpm":
+commands = []
+
+# Rawhide needs this because the keys used to sign packages are
+# cycled from time to time
if os_name == "Fedora" and os_version == "Rawhide":
-sys.stdout.write(textwrap.dedent("""
-RUN {package_manager} update -y --nogpgcheck
fedora-gpg-keys && \\
-{package_manager} update -y && \\
-{package_manager} install -y {pkgs} && \\
-{package_manager} autoremove -y && \\
-{package_manager} clean all -y
-""").format(**varmap))
-elif os_name == "CentOS":
-if os_version == "7":
-sys.stdout.write(textwrap.dedent("""
-RUN {package_manager} install -y epel-release && \\
-{package_manager} update -y && \\
-{package_manager} install -y {pkgs} && \\
-{package_manager} autoremove -y && \\
-{package_manager} clean all -y
-""").format(**varmap))
-else:
-sys.stdout.write(textwrap.dedent("""
-RUN {package_manager} install
'dnf-command(config-manager)' -y && \\
-{package_manager} config-manager --set-enabled
PowerTools -y && \\
-{package_manager} install -y epel-release && \\
-{package_manager} update -y && \\
-{package_manager} install -y {pkgs} && \\
-{package_manager} autoremove -y && \\
-{package_manager} clean all -y
-""").format(**varmap))
-else:
-sys.stdout.write(textwrap.dedent("""
-RUN {package_manager} update -y && \\
-{package_manager} install -y {pkgs} && \\
-{package_manager} autoremove -y && \\
-{package_manager} clean all -y
-""").format(**varmap))
+commands.extend([
+"{package_manager} update -y --nogpgcheck fedora-gpg-keys"
+])
+
+if os_name == "CentOS":
+# Starting with CentOS 8, most -devel packages are shipped in
+# the PowerTools repository, which is not enabled by default
+if os_version != "7":
+commands.extend([
+"{package_manager} install
'dnf-command(config-manager)' -y",
+"{package_manager} config-manager --set-enabled
PowerTools -y",
+])
+
+# Some of the packages we need are not part of CentOS proper
+# and are only available through EPEL
+commands.extend([
+"{package_manager} install -y epel-release",
+])
+
+commands.extend([
+"{package_manager} update -y",
+"{package_manager} install -y {pkgs}",
+"{package_manager} autoremove -y",
+"{package_manager} clean all -y",
+])
+
+script = "\nRUN " + (" && \\\n".join(commands)) + "\n"
+
+sys.stdout.write(script.format(**varmap))
if pip_pkgs:
sys.stdout.write(textwrap.dedent("""
--
2.23.0
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
