Re: [virt-manager 2/3] virt-manager: add support for adding panic device

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 




> -----Original Message-----
> From: Cole Robinson [mailto:crobinso@xxxxxxxxxx]
> Sent: Tuesday, January 07, 2014 4:32 AM
> To: Chen Hanxiao; virt-tools-list@xxxxxxxxxx
> Subject: Re:  [virt-manager 2/3] virt-manager: add
support for
> adding panic device
> 
> On 01/06/2014 03:04 AM, Chen Hanxiao wrote:
> > From: Chen Hanxiao <chenhanxiao@xxxxxxxxxxxxxx>
> >
> > Signed-off-by: Chen Hanxiao <chenhanxiao@xxxxxxxxxxxxxx>
> > ---
> >  ui/addhardware.ui          | 76
> ++++++++++++++++++++++++++++++++++++++++++++--
> >  virtManager/addhardware.py | 42 ++++++++++++++++++++++++-
> >  2 files changed, 115 insertions(+), 3 deletions(-)
> >
> 
> A played with this lightly. A few general comments:
> 
> - UI issues: the labels should be capitalized correctly, left aligned (set
> xalign to 0), and use underline/mnemonics.

v2 will change.

> 
> - Change the UI name of the device from 'PANIC' to 'Panic Notifier'. Might
> need to do the same in the other virt-manager patch
> 
v2 will change.

> - What's the point of iobase? When will a user ever want to change it? If
it's
> only rarely used, we might consider dropping the UI field for it.

If some device port conflict with pvpanic device, this para could help.
Consider that we already set the default value for it, I think keeping it
would not bring
troubles to users.

> 
> Thanks,
> Cole



_______________________________________________
virt-tools-list mailing list
virt-tools-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/virt-tools-list




[Index of Archives]     [Linux Virtualization]     [KVM Development]     [CentOS Virtualization]     [Netdev]     [Ethernet Bridging]     [Linux Wireless]     [Kernel Newbies]     [Security]     [Linux for Hams]     [Netfilter]     [Bugtraq]     [Yosemite Forum]     [MIPS Linux]     [ARM Linux]     [Linux RAID]     [Linux Admin]     [Samba]     [Video 4 Linux]

  Powered by Linux