On Wed, 2015-05-27 at 09:44 -0400, John Ferlan wrote:
> This should be split into two patches.... The first one dealing with
> just the error/bug and the second dealing with adding a panic device by
> default for PPC*
Thanks for the review, John!
I've split the changes into separate commits, as suggested, and added a
third one to improve the error message as well.
>Seems to be something that should documented in formatdomain.html.in -
>that is a panic device for PPC64 can not have an <address>
It certainly should :)
I've updated the documentation accordingly.
> > + goto error;
> > + }
> > +
> > if (def->panic->info.type == VIR_DOMAIN_DEVICE_ADDRESS_TYPE_ISA) {
> > virCommandAddArg(cmd, "-device");
> > virCommandAddArgFormat(cmd, "pvpanic,ioport=%d",
> > def->panic->info.addr.isa.iobase);
> > - } else if (def->panic->info.type ==
> > - VIR_DOMAIN_DEVICE_ADDRESS_TYPE_NONE) {
> > + } else if (def->panic->info.type == VIR_DOMAIN_DEVICE_ADDRESS_TYPE_NONE) {
>
> This change makes the line > 80 characters - so it should be changed
> back to two lines.
I've changed the if-else cascade into a switch statement, which keeps
the column count below 80 and looks better IMHO.
> Since this is being added as the default will there be issues with the
> virDomainPanicCheckABIStability()? That is for migration issues (and
> image save/restore type operations)?
To be honest, I don't know nearly enough about migration to be able to
tell whether this is the case. I'll look into it, but it would be great
if at least the other two commits (bug fix) could be merged in the
meantime.
I'll post a v3 right away.
Cheers.
--
Andrea Bolognani
Software Engineer - Virtualization Team
$ python -c "print('a'.join(['', 'bologn', '@redh', 't.com']))"
--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list