On 3/27/19 7:30 AM, Peter Krempa wrote:
On Wed, Mar 27, 2019 at 05:10:47 -0500, Eric Blake wrote:
Introduce a few more new virsh commands for performing backup jobs, as well as creating a checkpoint atomically with a snapshot.
At this time, I did not opt for a convenience command 'backup-begin-as' that cobbles together appropriate XML from the user's command line arguments, but that may be a viable future extension. Similarly, since backup is a potentially long-running operation, it might be nice to add some sugar that automatically handles waiting for the job to end, rather than making the user have to poll or figure out virsh event to do the same.
Signed-off-by: Eric Blake <eblake@redhat.com> --- tools/virsh-domain.c | 247 ++++++++++++++++++++++++++++++++++++++++- tools/virsh-snapshot.c | 37 +++++- tools/virsh.pod | 64 ++++++++++- 3 files changed, 337 insertions(+), 11 deletions(-)
diff --git a/tools/virsh-domain.c b/tools/virsh-domain.c index 1970710c07..4ae456146b 100644 --- a/tools/virsh-domain.c +++ b/tools/virsh-domain.c @@ -1,7 +1,7 @@ /* * virsh-domain.c: Commands to manage domain * - * Copyright (C) 2005, 2007-2016 Red Hat, Inc. + * Copyright (C) 2005-2019 Red Hat, Inc.
This seems wrong. It's adding dates in the past. Ideally you disable that script for libvirt altogether as it creates pointless churn.
I'm always wary of not updating copyrights, but I also see the complaints about possibly updating it incorrectly. Unless a lawyer gives me a strong reason behind one way or the other, it's obvious enough that many files have NOT been maintained after initial commit, so I can go ahead and flip the kill switch in my .emacs file for auto-copyrights to maintain status quo. I also don't mind scrubbing my series to remove the churn I've already caused, if that makes you feel better (but for files that my series DOES add, I will be listing dates starting from any file I copied from, through 2019).
* * This library is free software; you can redistribute it and/or * modify it under the terms of the GNU Lesser General Public
[1] Using 'check' instead of 'checkpoint' in variable names may be misleading below in multiple instances. It might evoke that it's supposed to be checked rather than referring to a checkpoint.
Fitting in 80 columns is harder with 'checkpoint', but I can do the rename for legibility.
+ +static bool +cmdBackupDumpXML(vshControl *ctl, + const vshCmd *cmd) +{ + virDomainPtr dom = NULL; + bool ret = false; + char *xml = NULL; + unsigned int flags = 0; + int id = 0; + + if (vshCommandOptBool(cmd, "security-info"))
This option is not mentioned in opts_backup_dumpxml.
Rebase leftovers; as there is no VIR_DOMAIN_BACKUP_XML_SECURE, this flag should be removed from the code.
+ flags |= VIR_DOMAIN_XML_SECURE;
And this flags setting is bogus.
unsigned int flags = 0; + VIR_AUTOFREE(char *check_buffer) = NULL;
VIR_AUTOFREE and legacy code is used inconsistently in this patch.
I can try to use the new stuff in more places (new code introduction is a good time; but this is also copy-and-paste from existing code; there's also the issue of timing delays, where the longer my patches are out-of-tree, the more the point they copied from has changed in the meantime). -- Eric Blake, Principal Software Engineer Red Hat, Inc. +1-919-301-3226 Virtualization: qemu.org | libvirt.org