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