> -----Original Message-----
> From: Cole Robinson [mailto:[email protected]]
> Sent: Tuesday, January 07, 2014 4:32 AM
> To: Chen Hanxiao; [email protected]
> Subject: Re: [virt-tools-list] [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 <[email protected]>
> >
> > Signed-off-by: Chen Hanxiao <[email protected]>
> > ---
> >  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
[email protected]
https://www.redhat.com/mailman/listinfo/virt-tools-list

Reply via email to