[virt-tools-list] [PATCH virt-manager] panic notifier: display default value if not set

[Chen Hanxiao]

> -----Original Message-----
> From: Cole Robinson [mailto:crobinso at redhat.com]
> Sent: Tuesday, January 14, 2014 7:59 AM
> To: Chen Hanxiao; virt-tools-list at redhat.com
> Subject: Re: [virt-tools-list] [PATCH virt-manager] panic notifier:
display default
> value if not set
> 
> On 01/13/2014 03:32 AM, Chen Hanxiao wrote:
> > From: Chen Hanxiao <chenhanxiao at cn.fujitsu.com>
> >
> > If we didn't set values for @type and @iobase in
> > XML, libvirt will use the default value.
> > Currently, virt-manager will display "-" if we don't
> > set any values.
> > This patch will use default values for display.
> > And add a test case to cover this scenario.
> >
> > Signed-off-by: Chen Hanxiao <chenhanxiao at cn.fujitsu.com>
> 
> 
> This test case is kinda redundant. In the existing panic test case, you
can
> just do:
> 
> check("iobase", "0x505", None, "0x506")
> 
> to verify we are correctly reading and setting None.
> 
> > diff --git a/virtManager/details.py b/virtManager/details.py
> > index 51573f6..d8481dd 100644
> > --- a/virtManager/details.py
> > +++ b/virtManager/details.py
> > @@ -3110,6 +3110,9 @@ class vmmDetails(vmmGObjectUI):
> >              widgetname = "panic-" + param.replace("_", "-")
> >              if not val:
> >                  val = getattr(dev, param)
> > +                if not val:
> > +                    propername = param.upper() + "_DEFAULT"
> > +                    val = (eval("virtinst.VirtualPanicDevice." +
> propername)).upper()
> >
> 
> I don't like eval() here. You can do
> 
> getattr(virtinst.VirtualPanicDevice, propername, "-").upper()
> 
> >              uihelpers.set_grid_row_visible(self.widget(widgetname),
> True)
> >              self.widget(widgetname).set_text(val or "-")
> > diff --git a/virtinst/devicepanic.py b/virtinst/devicepanic.py
> > index 5f7cbd8..a41af94 100644
> > --- a/virtinst/devicepanic.py
> > +++ b/virtinst/devicepanic.py
> > @@ -26,6 +26,7 @@ class VirtualPanicDevice(VirtualDevice):
> >
> >      virtual_device_type = VirtualDevice.VIRTUAL_DEV_PANIC
> >      ADDRESS_TYPE_ISA = "isa"
> > +    TYPE_DEFAULT = ADDRESS_TYPE_ISA
> >      TYPES = [ADDRESS_TYPE_ISA]
> >      IOBASE_DEFAULT = "0x505"
> >
> >
> 
> Otherwise looks fine.
> 

Thanks for the review.
v2 will come soon according to your comment.