On 09/21/2016 10:23 PM, Chen Hanxiao wrote:
> At 2016-09-22 05:30:48, "John Ferlan" <jfer...@redhat.com> wrote:
>> Rather than try to describe what I was thinking about for the passing
>> the compress program directly series from Chen Hanxiao:
>> I took the liberty of creating my own set of patches which should end
>> in the more or less same result.
>> The primary benefit is not calling virFileInPath twice since we already
>> get it at least once for the various compressed program callers, let's
>> pass what we originally found.
>> John Ferlan (7):
>> qemu: Move getCompressionType
>> qemu: Adjust doCoreDump to call getCompressionType
>> qemu: Introduce helper qemuGetCompressionProgram
>> qemu: Remove getCompressionType
>> qemu: Use qemuGetCompressionProgram for error paths
>> qemu: Remove qemuCompressProgramAvailable
>> qemu: Get/return compressedpath program
>> src/qemu/qemu_driver.c | 235
>> 1 file changed, 111 insertions(+), 124 deletions(-)
> Hi, John
> Looks that Patch 1, 3 and 4 should be scrash into one :).
I disagree - then in becomes one mish-mash of a patch which is harder to
review. The way I split things up (for me) was a logical progression of
1. Move code because we're about to use it from doCoreDump.
2. Use getCompressionType from within doCoreDump instead of passing it
3. Split out the "cfg->dumpImageFormat" into a separate helper and call
4. Remove need for getCompressionType for doCoreDump
As for the rest
5. Create new parameter to change how messaging is done. From doCoreDump
we can VIR_WARN since the code can continue; whereas, from
qemuDomainSaveFlags, qemuDomainManagedSave, and
qemuDomainSnapshotCreateActiveExternal we want to have the error and fail.
Using "ret < 0" in the error path allows us to know which of the two
Yes, the error message for snapshot will now be different, but that's
far more easily fixed by a new parameter than the extra code created in
your patch 2 to handle that error message.
6. Remove need for qemuCompressProgramAvailable by calling
virFindFileInPath directly. No need to worry about the
QEMU_SAVE_FORMAT_RAW case since it's already handled.
7. Return that compressed program name instead of calling
Again, since QEMU_SAVE_FORMAT_RAW was already handled we're good.
> Some other inline comments, Please check.
I'll post an adjustment for patch 5 error message tomorrow - it won't be
Tks for the comments/review -
> - Chen
libvir-list mailing list