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

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 guess I should wait for more feedback on the qemu front before committing to the final form of this proposed libvirt API.