On 02/27/2013 03:31 AM, Markus Armbruster wrote: > First pass, concentrating on interfaces, implementation mostly ignored. > > Dietmar Maurer <diet...@proxmox.com> writes: > >> We use a generic BackupDriver struct to encapsulate all archive format >> related function. >>
>> +# @backup: >> +# >> +# Starts a VM backup. >> +# >> +# @backup-file: the backup file name > > We'll need support for backup up to a fd passed with SCM rights. This already works with /dev/fdset/nnn for fds passed with 'add-fd', if you used qemu_open() on the passed file name. >> +# @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. > > 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. -- Eric Blake eblake redhat com +1-919-301-3266 Libvirt virtualization library http://libvirt.org
signature.asc
Description: OpenPGP digital signature