Re: [libvirt] [PATCH 09/11] qemu: blockPull: Refactor the rest of qemuDomainBlockJobImpl
On 04/09/2015 10:09 AM, Peter Krempa wrote:
...
>
> Just checking...
>
> This change is essentially the same as in qemuDomainBlockPullCommon
> where if (!modern) {} was added right?
Yes. This one would be redundant.
>
>> -
>> - if (backingName && mode != BLOCK_JOB_PULL) {
>> - virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
>> - _("backing name is supported only for block
pull"));
>> - return -1;
>> - }
>
> And this won't be necessary.... since we no longer have multiple
> (ab)users of the same API
Exactly.
>
>> -
>> - if (backingName && !base) {
>> - virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
>> - _("backing name requires a base image"));
>> - return -1;
>> - }
>
> Is there a check for this somewhere that I missed?
The caller ensures that this does not happen. We could leave this one
possibly in if you want.
Not a requirement (but at least you know I read the code ;-)). It's one
of those paranoia things.
John
>
>> + const char *cmd_name = modern ? "block-stream" :
"block_stream";
>>
>> - if (speed && mode == BLOCK_JOB_PULL && !modern) {
>> - virReportError(VIR_ERR_INTERNAL_ERROR,
>> - _("only modern block pull supports speed:
%llu"),
>> - speed);
>> - return -1;
>> - }
>
> And this is the second half of the check in qemuDomainBlockPullCommon
>
> ACK - in general - Just want to make sure the "if (backingName &&
!base)
> wasn't erroneously removed.
>
> John
Peter