[libvirt] [PATCHv3] qemu: Refactor qemuTranslateDiskSourcePool

Peter Krempa pkrempa at redhat.com
Tue Dec 3 09:51:17 UTC 2013


On 12/03/13 00:58, Eric Blake wrote:
> On 12/02/2013 08:19 AM, Peter Krempa wrote:
>> Before this patch, the translation function still needs a second ugly
>> helper function to actually format the command line for qemu. But if we
>> do the right stuff in the translation function, we don't have to bother
>> with the second function any more.
>>
>> This patch removes the messy qemuBuildVolumeString function and changes
>> qemuTranslateDiskSourcePool to set stuff up correctly so that the
>> regular code paths meant for volumes can be used to format the command
>> line correctly.
>>
>> For this purpose a new helper "qemuDiskGetActualType()" is introduced to
>> return the type of the volume in a pool.
>>
>> As a part of the refactor the qemuTranslateDiskSourcePool function is
>> fixed to do decisions based on the pool type instead of the volume type.
>> This allows to separate pool-type-specific stuff more clearly and will
>> ease addition of other pool types that will require certain other
>> operations to get the correct pool source.
>>
>> The previously fixed tests should make sure that we don't break stuff
>> that was working before.
>> ---
>>
> 
>> +                virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
>>                                 _("tray status 'open' is invalid for "
>> -                                 "block type disk"));
>> +                                 "block type %s"),
>> +                               disk->type == VIR_DOMAIN_DISK_TYPE_VOLUME ? _("volume") : _("disk"));
> 
> Translators generally prefer:
> 
> disk->type == VIR_DOMAIN_DISK_TYPE_VOLUME ?
> _("tray status 'open' is invalid for block type volume") :
> _("tray status 'open' is invalid for block type disk")

I went with this version

> 
> because there may be gender differences between volume and disk that
> require altering the rest of the sentence.
> 
> Looking good to me though; ACK.
> 

and pushed this one along with others already ACKed that were waiting on
this as a dependency.

Thanks for the review.

Peter


-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 901 bytes
Desc: OpenPGP digital signature
URL: <http://listman.redhat.com/archives/libvir-list/attachments/20131203/43a3b58c/attachment-0001.sig>


More information about the libvir-list mailing list