Re: [Qemu-devel] [PATCH v5 3/6] add backup related monitor commands

[Dietmar Maurer]

> >> +# @devlist: #optional list of block device names (separated by ',', ';'
> >> +# or ':'). By default the backup includes all writable block devices.
> >
> > Make this a proper list, please.
> 
> That is, make it a JSON array: '*devlist' : [ 'str' ] Any time that you pass 
> a string
> through JSON that then requires further ad-hoc parsing (such as splitting on 
> ',' or
> ':'), it is a sign that your JSON representation was incorrect.

I want to use that on HMP, so I need to parse ad-hoc anyways?

> > You bake the "only one backup at a time" restriction right into the API.
> > That's fine if and only if multiple backups cannot possibly make sense.
> >
> > Unless we agree that's the case, the API needs to be designed so it
> > can
> > grow: backup needs to return an ID (it already does), backup-cancel
> > needs to take it as argument (it doesn't), and query-backup either
> > returns a list, or takes an ID argument.
> 
> Agreed.  In the case of backup-cancel, if you want to make the argument
> optional, and have it do the right thing when only one job is active (and 
> only fail
> if multiple jobs are running), that would also work.
> 
> >
> > The implementation may still restrict to a single backup, of course.
> 
> The initial implementation, obviously.  The point is that the API should 
> allow for
> growth, even if the initial implementation doesn't support everything that the
> API allows.

You can easily extend the API to allow multiple backups (In a fully backwards
compatible way). So there is no need to change that now.