| Message ID | 1375427079-16822-1-git-send-email-hutao@cn.fujitsu.com |
|---|---|
| State | New |
| Headers | show |
On 08/02/2013 09:04 AM, Hu Tao wrote: > The problem with pvpanic being an internal device is that VMs running > operating systems without a driver for this device will have problems > when qemu will be upgraded (from qemu without this pvpanic). > > The outcome may be, for example: in Windows(let's say XP) the Device > manager will open a "new device" wizard and the device will appear as > an unrecognized device. On a cluster with hundreds of such VMs, If > that cluster has a health monitoring service it may show all the VMs > in a "not healthy" state. > > Reported-by: Marcel Apfelbaum <marcel.a@redhat.com> > Signed-off-by: Hu Tao <hutao@cn.fujitsu.com> NACK, this is premature. It is fundamentally a firmware problem. We have time to apply an even smaller patch that doesn't set has_pvpanic to true, and delay the whole feature to 1.7, if we do not fix the firmware in the next two weeks. Paolo
On Fri, Aug 02, 2013 at 10:27:31AM +0200, Paolo Bonzini wrote: > On 08/02/2013 09:04 AM, Hu Tao wrote: > >The problem with pvpanic being an internal device is that VMs running > >operating systems without a driver for this device will have problems > >when qemu will be upgraded (from qemu without this pvpanic). > > > >The outcome may be, for example: in Windows(let's say XP) the Device > >manager will open a "new device" wizard and the device will appear as > >an unrecognized device. On a cluster with hundreds of such VMs, If > >that cluster has a health monitoring service it may show all the VMs > >in a "not healthy" state. > > > >Reported-by: Marcel Apfelbaum <marcel.a@redhat.com> > >Signed-off-by: Hu Tao <hutao@cn.fujitsu.com> > > NACK, > > this is premature. It is fundamentally a firmware problem. > > We have time to apply an even smaller patch that doesn't set > has_pvpanic to true, and delay the whole feature to 1.7, if we do > not fix the firmware in the next two weeks. > > Paolo I think this is not just a firmware problem. Adding device by default was too rush, assumption was risk of guest bugs was 0. We are now seeing problems with bios guest code and with linux guest drivers as well. Yes they all can be fixed, but we simply shouldn't force this risk of broken guests on everyone. libvirt is the main user and libvirt people indicated their preference to creating device with -device pvpanic rather than a built-in one that can't be removed. So please reconsider, and here's an ack from me. Acked-by: Michael S. Tsirkin <mst@redhat.com>
Am 11.08.2013 12:33, schrieb Michael S. Tsirkin: > On Fri, Aug 02, 2013 at 10:27:31AM +0200, Paolo Bonzini wrote: >> On 08/02/2013 09:04 AM, Hu Tao wrote: >>> The problem with pvpanic being an internal device is that VMs running >>> operating systems without a driver for this device will have problems >>> when qemu will be upgraded (from qemu without this pvpanic). >>> >>> The outcome may be, for example: in Windows(let's say XP) the Device >>> manager will open a "new device" wizard and the device will appear as >>> an unrecognized device. On a cluster with hundreds of such VMs, If >>> that cluster has a health monitoring service it may show all the VMs >>> in a "not healthy" state. >>> >>> Reported-by: Marcel Apfelbaum <marcel.a@redhat.com> >>> Signed-off-by: Hu Tao <hutao@cn.fujitsu.com> >> >> NACK, >> >> this is premature. It is fundamentally a firmware problem. >> >> We have time to apply an even smaller patch that doesn't set >> has_pvpanic to true, and delay the whole feature to 1.7, if we do >> not fix the firmware in the next two weeks. >> >> Paolo > > I think this is not just a firmware problem. Adding device by default > was too rush, assumption was risk of guest bugs was 0. > > We are now seeing problems with bios guest code and with linux guest > drivers as well. Yes they all can be fixed, but we simply shouldn't > force this risk of broken guests on everyone. > > libvirt is the main user and libvirt people > indicated their preference to creating device with > -device pvpanic rather than a built-in one that > can't be removed. > > So please reconsider, and here's an ack from me. > > Acked-by: Michael S. Tsirkin <mst@redhat.com> NACK for this v1: As pointed out on the KVM call, we still need to keep the pvpanic device around by default for pc-*-1.5. Removing has_pvpanic completely therefore seems wrong. Can you submit a v2 for rc3 tomorrow? Andreas
On Sun, Aug 11, 2013 at 04:45:03PM +0200, Andreas Färber wrote: > Am 11.08.2013 12:33, schrieb Michael S. Tsirkin: > > On Fri, Aug 02, 2013 at 10:27:31AM +0200, Paolo Bonzini wrote: > >> On 08/02/2013 09:04 AM, Hu Tao wrote: > >>> The problem with pvpanic being an internal device is that VMs running > >>> operating systems without a driver for this device will have problems > >>> when qemu will be upgraded (from qemu without this pvpanic). > >>> > >>> The outcome may be, for example: in Windows(let's say XP) the Device > >>> manager will open a "new device" wizard and the device will appear as > >>> an unrecognized device. On a cluster with hundreds of such VMs, If > >>> that cluster has a health monitoring service it may show all the VMs > >>> in a "not healthy" state. > >>> > >>> Reported-by: Marcel Apfelbaum <marcel.a@redhat.com> > >>> Signed-off-by: Hu Tao <hutao@cn.fujitsu.com> > >> > >> NACK, > >> > >> this is premature. It is fundamentally a firmware problem. > >> > >> We have time to apply an even smaller patch that doesn't set > >> has_pvpanic to true, and delay the whole feature to 1.7, if we do > >> not fix the firmware in the next two weeks. > >> > >> Paolo > > > > I think this is not just a firmware problem. Adding device by default > > was too rush, assumption was risk of guest bugs was 0. > > > > We are now seeing problems with bios guest code and with linux guest > > drivers as well. Yes they all can be fixed, but we simply shouldn't > > force this risk of broken guests on everyone. > > > > libvirt is the main user and libvirt people > > indicated their preference to creating device with > > -device pvpanic rather than a built-in one that > > can't be removed. > > > > So please reconsider, and here's an ack from me. > > > > Acked-by: Michael S. Tsirkin <mst@redhat.com> > > NACK for this v1: As pointed out on the KVM call, we still need to keep > the pvpanic device around by default for pc-*-1.5. Removing has_pvpanic > completely therefore seems wrong. We also mentioned an option to patch 1.5 stable to change it there, but I'm fine with not doing it. > Can you submit a v2 for rc3 tomorrow? > > Andreas > -- > SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany > GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg
On Sun, 2013-08-11 at 16:45 +0200, Andreas Färber wrote: > Am 11.08.2013 12:33, schrieb Michael S. Tsirkin: > > On Fri, Aug 02, 2013 at 10:27:31AM +0200, Paolo Bonzini wrote: > >> On 08/02/2013 09:04 AM, Hu Tao wrote: > >>> The problem with pvpanic being an internal device is that VMs running > >>> operating systems without a driver for this device will have problems > >>> when qemu will be upgraded (from qemu without this pvpanic). > >>> > >>> The outcome may be, for example: in Windows(let's say XP) the Device > >>> manager will open a "new device" wizard and the device will appear as > >>> an unrecognized device. On a cluster with hundreds of such VMs, If > >>> that cluster has a health monitoring service it may show all the VMs > >>> in a "not healthy" state. > >>> > >>> Reported-by: Marcel Apfelbaum <marcel.a@redhat.com> > >>> Signed-off-by: Hu Tao <hutao@cn.fujitsu.com> > >> > >> NACK, > >> > >> this is premature. It is fundamentally a firmware problem. > >> > >> We have time to apply an even smaller patch that doesn't set > >> has_pvpanic to true, and delay the whole feature to 1.7, if we do > >> not fix the firmware in the next two weeks. > >> > >> Paolo > > > > I think this is not just a firmware problem. Adding device by default > > was too rush, assumption was risk of guest bugs was 0. > > > > We are now seeing problems with bios guest code and with linux guest > > drivers as well. Yes they all can be fixed, but we simply shouldn't > > force this risk of broken guests on everyone. > > > > libvirt is the main user and libvirt people > > indicated their preference to creating device with > > -device pvpanic rather than a built-in one that > > can't be removed. > > > > So please reconsider, and here's an ack from me. > > > > Acked-by: Michael S. Tsirkin <mst@redhat.com> > > NACK for this v1: As pointed out on the KVM call, we still need to keep > the pvpanic device around by default for pc-*-1.5. Removing has_pvpanic > completely therefore seems wrong. Can you submit a v2 for rc3 tomorrow? I just sent a patchset with V2. Can you please review it? Thanks, Marcel > > Andreas >
diff --git a/hw/i386/pc_piix.c b/hw/i386/pc_piix.c index ab25458..3ccf96c 100644 --- a/hw/i386/pc_piix.c +++ b/hw/i386/pc_piix.c @@ -56,7 +56,6 @@ static const int ide_iobase[MAX_IDE_BUS] = { 0x1f0, 0x170 }; static const int ide_iobase2[MAX_IDE_BUS] = { 0x3f6, 0x376 }; static const int ide_irq[MAX_IDE_BUS] = { 14, 15 }; -static bool has_pvpanic = true; static bool has_pci_info = true; /* PC hardware initialisation */ @@ -228,10 +227,6 @@ static void pc_init1(MemoryRegion *system_memory, if (pci_enabled) { pc_pci_device_init(pci_bus); } - - if (has_pvpanic) { - pvpanic_init(isa_bus); - } } static void pc_init_pci(QEMUMachineInitArgs *args) @@ -257,7 +252,6 @@ static void pc_init_pci_1_5(QEMUMachineInitArgs *args) static void pc_init_pci_1_4(QEMUMachineInitArgs *args) { - has_pvpanic = false; x86_cpu_compat_set_features("n270", FEAT_1_ECX, 0, CPUID_EXT_MOVBE); pc_init_pci_1_5(args); } @@ -290,7 +284,6 @@ static void pc_init_pci_no_kvmclock(QEMUMachineInitArgs *args) const char *kernel_cmdline = args->kernel_cmdline; const char *initrd_filename = args->initrd_filename; const char *boot_device = args->boot_device; - has_pvpanic = false; has_pci_info = false; disable_kvm_pv_eoi(); enable_compat_apic_id_mode(); @@ -309,7 +302,6 @@ static void pc_init_isa(QEMUMachineInitArgs *args) const char *kernel_cmdline = args->kernel_cmdline; const char *initrd_filename = args->initrd_filename; const char *boot_device = args->boot_device; - has_pvpanic = false; has_pci_info = false; if (cpu_model == NULL) cpu_model = "486"; diff --git a/hw/i386/pc_q35.c b/hw/i386/pc_q35.c index 2f35d12..c816c2f 100644 --- a/hw/i386/pc_q35.c +++ b/hw/i386/pc_q35.c @@ -46,7 +46,6 @@ /* ICH9 AHCI has 6 ports */ #define MAX_SATA_PORTS 6 -static bool has_pvpanic = true; static bool has_pci_info = true; /* PC hardware initialisation */ @@ -211,10 +210,6 @@ static void pc_q35_init(QEMUMachineInitArgs *args) if (pci_enabled) { pc_pci_device_init(host_bus); } - - if (has_pvpanic) { - pvpanic_init(isa_bus); - } } static void pc_q35_init_1_5(QEMUMachineInitArgs *args) @@ -225,7 +220,6 @@ static void pc_q35_init_1_5(QEMUMachineInitArgs *args) static void pc_q35_init_1_4(QEMUMachineInitArgs *args) { - has_pvpanic = false; x86_cpu_compat_set_features("n270", FEAT_1_ECX, 0, CPUID_EXT_MOVBE); pc_q35_init_1_5(args); }
The problem with pvpanic being an internal device is that VMs running operating systems without a driver for this device will have problems when qemu will be upgraded (from qemu without this pvpanic). The outcome may be, for example: in Windows(let's say XP) the Device manager will open a "new device" wizard and the device will appear as an unrecognized device. On a cluster with hundreds of such VMs, If that cluster has a health monitoring service it may show all the VMs in a "not healthy" state. Reported-by: Marcel Apfelbaum <marcel.a@redhat.com> Signed-off-by: Hu Tao <hutao@cn.fujitsu.com> --- hw/i386/pc_piix.c | 8 -------- hw/i386/pc_q35.c | 6 ------ 2 files changed, 14 deletions(-)