Re: [libvirt] [PATCH 0/7] Pass the path of compress program (redo)

On 09/21/2016 10:23 PM, Chen Hanxiao wrote: 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 this series. 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 options failed. 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 qemuCompressProgramName again. Again, since QEMU_SAVE_FORMAT_RAW was already handled we're good. I'll post an adjustment for patch 5 error message tomorrow - it won't be complicated. Tks for the comments/review - John