[libvirt] [PATCHv4 16/18] blockjob: add virDomainBlockCopy

[Jiri Denemark]

On Fri, Apr 13, 2012 at 15:42:20 -0600, Eric Blake wrote:
> On 04/13/2012 03:25 PM, Jiri Denemark wrote:
> > On Mon, Apr 09, 2012 at 21:52:25 -0600, Eric Blake wrote:
> >> This new API provides additional flexibility over what can be
> >> crammed on top of virDomainBlockRebase (namely, the ability to
> >> specify an arbitrary destination format, for things like copying
> >> qcow2 into qed without having to pre-create the destination), at
> >> the expense that it cannot be backported without bumping the .so
> >> version.
> >>
> 
> >> +typedef enum {
> >> +    VIR_DOMAIN_BLOCK_COPY_SHALLOW   = 1 << 0, /* Limit copy to top of source
> >> +                                                 backing chain */
> >> +    VIR_DOMAIN_BLOCK_COPY_REUSE_EXT = 1 << 1, /* Reuse existing external
> >> +                                                 file for a copy */
> >> +} virDomainBlockCopyFlags;
> > 
> > Hmm, we have several flags enums that end up being passed to a single internal
> > API and one has to be extra careful when adding new flags. Should we make a
> > note about this to the affected enums?
> 
> Yes, a note would be helpful (it's only the two least-significant bits
> that we are playing fast and loose with at the moment, but if we ever
> expand to a third common bit, I see where it could get confusing).
> 
> >>
> >>  /**
> >> + * virDomainBlockCopy:
> >> + * @dom: pointer to domain object
> >> + * @disk: path to the block device, or device shorthand
> >> + * @dest: path to the copy destination
> >> + * @format: format of the destination
> >> + * @bandwidth: (optional) specify copy bandwidth limit in Mbps
> >> + * @flags: bitwise-OR of virDomainBlockCopyFlags
> > 
> > OK, so this new API may be used to avoid format guessing involved in
> > virDomainBlockRebase. Shouldn't we introduce an enhanced version of
> > virDomainBlockRebase with format parameter instead of introducing an API with
> > a different name that does almost the same as virDomainBlockRebase?
> 
> And what would you name it?  I'm saying that virDomainBlockCopy _is_ an
> enhanced virDomainBlockRebase, and the name BlockCopy was the name I
> picked, as it looks nicer than virDomainBlockRebase2().

I don't know, I was probably expecting something like virDomainBlockRebaseExt
:-P I'm just missing a clear link between virDomainBlockRebase and
virDomainBlockCopy. I guess a note to virDomainBlockRebase documentation
mentioning virDomainBlockCopy as an enhanced version would work too.

> I guess I should wait for more feedback on the qemu front before
> committing to the final form of this proposed libvirt API.

Yeah, I think that's wise.

Jirka