Re: [libvirt] [PATCHv5 19/19] qemu: Add support for networked disks for block pull/block rebase

On 06/19/2014 07:59 AM, Peter Krempa wrote: > Now that we are able to select images from the backing chain via indexed > access we should also convert possible network sources to > qemu-compatible strings before passing them to qemu. > --- > src/qemu/qemu_driver.c | 45 +++++++++++++++++++++++++++++++++++++++++---- > 1 file changed, 41 insertions(+), 4 deletions(-) Same caveats as in 18/19 about not necessarily working in mixed-source chains (for that, we'd need to use node-names); but as it is definitely more powerful than what libvirt previously supported, it's still worth including under the incremental improvement umbrella. > @@ -15040,6 +15042,13 @@ qemuDomainBlockJobImpl(virDomainObjPtr vm, > goto cleanup; > } > > + if (flags & VIR_DOMAIN_BLOCK_REBASE_RELATIVE && !base) { > + virReportError(VIR_ERR_INVALID_ARG, "%s", > + _("flag VIR_DOMAIN_BLOCK_REBASE_RELATIVE is valid only " > + " with non-null base ")); Trailing space in the error message. This treats relative name with no base as a hard error, which is okay but should be documented. > + > + if (!backingPath) { > + virReportError(VIR_ERR_OPERATION_INVALID, "%s", > + _("Can't keep relative backing relationship.")); No trailing '.'. Once again, back to the question of whether it is nicer for the flag to be advisory (best effort to use relative, but absolute fallback is okay) or mandatory (fail if the request cannot be honored). At this point, I'm leaning towards mandatory (it's easier to relax mandatory to advisory later than it is to give advisory now and tighten it up later; and I like to know if my explicit request cannot be honored). But the documentation needs to match what we choose, and it would help to have Adam's insight as a client of this flag.