Re: [libvirt] [PATCHv3] qemu: Refactor qemuTranslateDiskSourcePool

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")

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