On Wed, Nov 25, 2009 at 11:07:26AM +0000, Michael Hanselmann wrote:
> 2009/11/25 Iustin Pop <[email protected]>:
> > On Wed, Nov 25, 2009 at 10:59:14AM +0000, Michael Hanselmann wrote:
> >> 2009/11/25 Iustin Pop <[email protected]>:
> >> > --- a/lib/hypervisor/hv_kvm.py
> >> > +++ b/lib/hypervisor/hv_kvm.py
> >> > +    # Cache mode
> >> > +    disk_cache = hvp[constants.HV_DISK_CACHE]
> >> > +    if disk_cache != constants.HT_CACHE_DEFAULT:
> >> > +      cache_val = ",cache=%s" % disk_cache
> >> > +    else:
> >> > +      cache_val = ""
> >> >     for cfdev, dev_path in block_devices:
> >> >       if cfdev.mode != constants.DISK_RDWR:
> >> >         raise errors.HypervisorError("Instance has read-only disks which"
> >> > @@ -323,7 +331,8 @@ class KVMHypervisor(hv_base.BaseHypervisor):
> >> >       else:
> >> >         boot_val = ''
> >> >
> >> > -      drive_val = 'file=%s,format=raw%s%s' % (dev_path, if_val, 
> >> > boot_val)
> >> > +      drive_val = 'file=%s,format=raw%s%s%s' % (dev_path, if_val, 
> >> > boot_val,
> >> > +                                                cache_val)
> >>
> >> I think this would be a good place to use a list and ",".join.
> >
> > Well, not quite, because cache_val=="" means we don't want to add ',,'
> > in the string. All *_val values can be empty.
> 
> Yes (just an example and not for 2.1 anyway):
> 
> drive_vals = ["file=%s" % dev_path, "format=raw"]
> …
> if …:
>   drive_vals.append("if=%s" % …)
> 
> if disk_cache != constants.HT_CACHE_DEFAULT:
>   drive_vals.append("cache=%s" % disk_cache)
> …
> drive_val = ",".join(drive_vals)

Ah OK, now I understand what you mean.

iustin

Reply via email to