Re: [libvirt] [PATCH v7 13/23] backup: Implement backup APIs for remote driver

On 3/27/19 6:36 AM, Daniel P. Berrangé wrote: > On Wed, Mar 27, 2019 at 05:10:44AM -0500, Eric Blake wrote: >> The remote code generator had to be taught about the new >> virDomainCheckpointPtr type, and about the capitalization of >> virDomainSnapshotCreateXML2(), at which point the remote driver code >> for backups can be generated. >> >> Signed-off-by: Eric Blake <eblake(a)redhat.com&gt; >> --- >> src/remote/remote_daemon_dispatch.c | 22 ++- >> src/remote/remote_driver.c | 34 +++- >> src/remote/remote_protocol.x | 259 +++++++++++++++++++++++++++- >> src/remote_protocol-structs | 139 +++++++++++++++ >> src/rpc/gendispatch.pl | 34 ++-- >> 5 files changed, 468 insertions(+), 20 deletions(-) > > >> +struct remote_domain_backup_begin_args { >> + remote_nonnull_domain dom; >> + remote_string disk_xml; >> + remote_string checkpoint_xml; >> + unsigned int flags; >> +}; >> + >> +struct remote_domain_backup_begin_ret { >> + int result; > > Feels like this should be 'id', since IIUC it is returning a job > ID. Can swap (generator should handle it just fine) >> +}; > > With that one rename above > > Reviewed-by: Daniel P. Berrangé <berrange(a)redhat.com&gt; This was the patch where I had a question on ACL semantics: + * @generate: both + * @acl: domain:snapshot + * @acl: domain:fs_freeze:VIR_DOMAIN_SNAPSHOT_CREATE_QUIESCE + */ + REMOTE_PROC_DOMAIN_SNAPSHOT_CREATE_XML2 = 418 Is there an easy way to make this also do a check against @acl: domain:checkpoint, but only when the checkpointXml flag is non-NULL? The quickest I could think of would be automatically setting a VIR_DOMAIN_SNAPSHOT_CREATE_CHECKPOINT flag that libvirt-domain-snapshot.c automatically sets if checkpointXml is clear, so that I could then write @acl: domain:checkpoint:VIR_DOMAIN_SNAPSHOT_CREATE_CHECKPOINT. We've set witness flags in other situations (such as VIR_TYPED_PARAM_STRING_OKAY), but never for the purpose of an ACL check. On the other hand, tweaking the ACL code generator to take more than just 'flags' for doing checks seems like a lot of work for very little gain.