[libvirt] [PATCH v7 16/23] backup: Implement virsh support for backup

[Eric Blake]

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 at 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

-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 488 bytes
Desc: OpenPGP digital signature
URL: <http://listman.redhat.com/archives/libvir-list/attachments/20190327/b5d8dafe/attachment-0001.sig>