Re: [Qemu-devel] [PATCH for-1.4] e1000: unbreak the guest network migration to 1.3

2013-02-15 Thread Stefan Hajnoczi 
On Thu, Feb 14, 2013 at 07:15:03PM +0200, Michael S. Tsirkin wrote:
 QEMU 1.3 does not emulate the link auto negotiation, so if migrate to a
 1.3 machine during link auto negotiation, the guest link will be set to down.
 Fix this by just disabling auto negotiation for 1.3.
 
 Signed-off-by: Michael S. Tsirkin m...@redhat.com
 ---
  hw/e1000.c   | 25 +
  hw/pc_piix.c |  4 
  2 files changed, 29 insertions(+)

Besides the naming issues that have been pointed out:

Reviewed-by: Stefan Hajnoczi stefa...@redhat.com

[Qemu-devel] [PATCH for-1.4] e1000: unbreak the guest network migration to 1.3

2013-02-14 Thread Michael S. Tsirkin 
QEMU 1.3 does not emulate the link auto negotiation, so if migrate to a
1.3 machine during link auto negotiation, the guest link will be set to down.
Fix this by just disabling auto negotiation for 1.3.

Signed-off-by: Michael S. Tsirkin m...@redhat.com
---
 hw/e1000.c   | 25 +
 hw/pc_piix.c |  4 
 2 files changed, 29 insertions(+)

diff --git a/hw/e1000.c b/hw/e1000.c
index d6fe815..263f2f4 100644
--- a/hw/e1000.c
+++ b/hw/e1000.c
@@ -131,6 +131,11 @@ typedef struct E1000State_st {
 } eecd_state;
 
 QEMUTimer *autoneg_timer;
+
+/* Enabled compatibility for migration to/from rhel6.3.0 and older */
+#define E1000_FLAG_QEMU_13_BIT 0
+#define E1000_FLAG_QEMU_13 (1  E1000_FLAG_QEMU_13_BIT)
+uint32_t compat_flags;
 } E1000State;
 
 #definedefreg(x)   x = (E1000_##x2)
@@ -165,6 +170,14 @@ e1000_link_up(E1000State *s)
 static void
 set_phy_ctrl(E1000State *s, int index, uint16_t val)
 {
+/*
+ * QEMU 1.3 does not support link auto-negotiation emulation, so if we
+ * migrate during auto negotiation, after migration the link will be
+ * down.
+ */
+if (s-compat_flags  E1000_FLAG_QEMU_13) {
+return;
+}
 if ((val  MII_CR_AUTO_NEG_EN)  (val  MII_CR_RESTART_AUTO_NEG)) {
 e1000_link_down(s);
 s-phy_reg[PHY_STATUS] = ~MII_SR_AUTONEG_COMPLETE;
@@ -1120,6 +1133,11 @@ static void e1000_pre_save(void *opaque)
 {
 E1000State *s = opaque;
 NetClientState *nc = qemu_get_queue(s-nic);
+
+if (s-compat_flags  E1000_FLAG_QEMU_13) {
+return;
+}
+
 /*
  * If link is down and auto-negotiation is ongoing, complete
  * auto-negotiation immediately.  This allows is to look at
@@ -1141,6 +1159,11 @@ static int e1000_post_load(void *opaque, int version_id)
  * to link status bit in mac_reg[STATUS].
  * Alternatively, restart link negotiation if it was in progress. */
 nc-link_down = (s-mac_reg[STATUS]  E1000_STATUS_LU) == 0;
+
+if (s-compat_flags  E1000_FLAG_QEMU_13) {
+return 0;
+}
+
 if (s-phy_reg[PHY_CTRL]  MII_CR_AUTO_NEG_EN 
 s-phy_reg[PHY_CTRL]  MII_CR_RESTART_AUTO_NEG 
 !(s-phy_reg[PHY_STATUS]  MII_SR_AUTONEG_COMPLETE)) {
@@ -1343,6 +1366,8 @@ static void qdev_e1000_reset(DeviceState *dev)
 
 static Property e1000_properties[] = {
 DEFINE_NIC_PROPERTIES(E1000State, conf),
+DEFINE_PROP_BIT(x-qemu_13_compat, E1000State,
+compat_flags, E1000_FLAG_QEMU_13, false),
 DEFINE_PROP_END_OF_LIST(),
 };
 
diff --git a/hw/pc_piix.c b/hw/pc_piix.c
index ba09714..89a3f1f 100644
--- a/hw/pc_piix.c
+++ b/hw/pc_piix.c
@@ -537,6 +537,10 @@ static QEMUMachine pc_machine_v0_13 = {
 .driver   = vmware-svga,
 .property = rombar,
 .value= stringify(0),
+},{
+.driver   = e1000,
+.property = x-qemu_13_compat,
+.value= on,
 },
 { /* end of list */ }
 },
-- 
MST

Re: [Qemu-devel] [PATCH for-1.4] e1000: unbreak the guest network migration to 1.3

2013-02-14 Thread Paolo Bonzini 
Il 14/02/2013 18:15, Michael S. Tsirkin ha scritto:
 QEMU 1.3 does not emulate the link auto negotiation, so if migrate to a
 1.3 machine during link auto negotiation, the guest link will be set to down.
 Fix this by just disabling auto negotiation for 1.3.
 
 Signed-off-by: Michael S. Tsirkin m...@redhat.com

A nicer property name perhaps would be preferrable, but it's fine if
Anthony doesn't want to let the release slip.

Paolo

 ---
  hw/e1000.c   | 25 +
  hw/pc_piix.c |  4 
  2 files changed, 29 insertions(+)
 
 diff --git a/hw/e1000.c b/hw/e1000.c
 index d6fe815..263f2f4 100644
 --- a/hw/e1000.c
 +++ b/hw/e1000.c
 @@ -131,6 +131,11 @@ typedef struct E1000State_st {
  } eecd_state;
  
  QEMUTimer *autoneg_timer;
 +
 +/* Enabled compatibility for migration to/from rhel6.3.0 and older */
 +#define E1000_FLAG_QEMU_13_BIT 0
 +#define E1000_FLAG_QEMU_13 (1  E1000_FLAG_QEMU_13_BIT)
 +uint32_t compat_flags;
  } E1000State;
  
  #define  defreg(x)   x = (E1000_##x2)
 @@ -165,6 +170,14 @@ e1000_link_up(E1000State *s)
  static void
  set_phy_ctrl(E1000State *s, int index, uint16_t val)
  {
 +/*
 + * QEMU 1.3 does not support link auto-negotiation emulation, so if we
 + * migrate during auto negotiation, after migration the link will be
 + * down.
 + */
 +if (s-compat_flags  E1000_FLAG_QEMU_13) {
 +return;
 +}
  if ((val  MII_CR_AUTO_NEG_EN)  (val  MII_CR_RESTART_AUTO_NEG)) {
  e1000_link_down(s);
  s-phy_reg[PHY_STATUS] = ~MII_SR_AUTONEG_COMPLETE;
 @@ -1120,6 +1133,11 @@ static void e1000_pre_save(void *opaque)
  {
  E1000State *s = opaque;
  NetClientState *nc = qemu_get_queue(s-nic);
 +
 +if (s-compat_flags  E1000_FLAG_QEMU_13) {
 +return;
 +}
 +
  /*
   * If link is down and auto-negotiation is ongoing, complete
   * auto-negotiation immediately.  This allows is to look at
 @@ -1141,6 +1159,11 @@ static int e1000_post_load(void *opaque, int 
 version_id)
   * to link status bit in mac_reg[STATUS].
   * Alternatively, restart link negotiation if it was in progress. */
  nc-link_down = (s-mac_reg[STATUS]  E1000_STATUS_LU) == 0;
 +
 +if (s-compat_flags  E1000_FLAG_QEMU_13) {
 +return 0;
 +}
 +
  if (s-phy_reg[PHY_CTRL]  MII_CR_AUTO_NEG_EN 
  s-phy_reg[PHY_CTRL]  MII_CR_RESTART_AUTO_NEG 
  !(s-phy_reg[PHY_STATUS]  MII_SR_AUTONEG_COMPLETE)) {
 @@ -1343,6 +1366,8 @@ static void qdev_e1000_reset(DeviceState *dev)
  
  static Property e1000_properties[] = {
  DEFINE_NIC_PROPERTIES(E1000State, conf),
 +DEFINE_PROP_BIT(x-qemu_13_compat, E1000State,
 +compat_flags, E1000_FLAG_QEMU_13, false),
  DEFINE_PROP_END_OF_LIST(),
  };
  
 diff --git a/hw/pc_piix.c b/hw/pc_piix.c
 index ba09714..89a3f1f 100644
 --- a/hw/pc_piix.c
 +++ b/hw/pc_piix.c
 @@ -537,6 +537,10 @@ static QEMUMachine pc_machine_v0_13 = {
  .driver   = vmware-svga,
  .property = rombar,
  .value= stringify(0),
 +},{
 +.driver   = e1000,
 +.property = x-qemu_13_compat,
 +.value= on,
  },
  { /* end of list */ }
  },

Re: [Qemu-devel] [PATCH for-1.4] e1000: unbreak the guest network migration to 1.3

2013-02-14 Thread Anthony Liguori 
Michael S. Tsirkin m...@redhat.com writes:

 QEMU 1.3 does not emulate the link auto negotiation, so if migrate to a
 1.3 machine during link auto negotiation, the guest link will be set to down.
 Fix this by just disabling auto negotiation for 1.3.

 Signed-off-by: Michael S. Tsirkin m...@redhat.com
 ---
  hw/e1000.c   | 25 +
  hw/pc_piix.c |  4 
  2 files changed, 29 insertions(+)

 diff --git a/hw/e1000.c b/hw/e1000.c
 index d6fe815..263f2f4 100644
 --- a/hw/e1000.c
 +++ b/hw/e1000.c
 @@ -131,6 +131,11 @@ typedef struct E1000State_st {
  } eecd_state;
  
  QEMUTimer *autoneg_timer;
 +
 +/* Enabled compatibility for migration to/from rhel6.3.0 and older */

I presume you mean QEMU 1.3...

 +#define E1000_FLAG_QEMU_13_BIT 0
 +#define E1000_FLAG_QEMU_13 (1  E1000_FLAG_QEMU_13_BIT)
 +uint32_t compat_flags;
  } E1000State;
  
  #define  defreg(x)   x = (E1000_##x2)
 @@ -165,6 +170,14 @@ e1000_link_up(E1000State *s)
  static void
  set_phy_ctrl(E1000State *s, int index, uint16_t val)
  {
 +/*
 + * QEMU 1.3 does not support link auto-negotiation emulation, so if we
 + * migrate during auto negotiation, after migration the link will be
 + * down.
 + */
 +if (s-compat_flags  E1000_FLAG_QEMU_13) {
 +return;
 +}
  if ((val  MII_CR_AUTO_NEG_EN)  (val  MII_CR_RESTART_AUTO_NEG)) {
  e1000_link_down(s);
  s-phy_reg[PHY_STATUS] = ~MII_SR_AUTONEG_COMPLETE;
 @@ -1120,6 +1133,11 @@ static void e1000_pre_save(void *opaque)
  {
  E1000State *s = opaque;
  NetClientState *nc = qemu_get_queue(s-nic);
 +
 +if (s-compat_flags  E1000_FLAG_QEMU_13) {
 +return;
 +}
 +
  /*
   * If link is down and auto-negotiation is ongoing, complete
   * auto-negotiation immediately.  This allows is to look at
 @@ -1141,6 +1159,11 @@ static int e1000_post_load(void *opaque, int 
 version_id)
   * to link status bit in mac_reg[STATUS].
   * Alternatively, restart link negotiation if it was in progress. */
  nc-link_down = (s-mac_reg[STATUS]  E1000_STATUS_LU) == 0;
 +
 +if (s-compat_flags  E1000_FLAG_QEMU_13) {
 +return 0;
 +}
 +
  if (s-phy_reg[PHY_CTRL]  MII_CR_AUTO_NEG_EN 
  s-phy_reg[PHY_CTRL]  MII_CR_RESTART_AUTO_NEG 
  !(s-phy_reg[PHY_STATUS]  MII_SR_AUTONEG_COMPLETE)) {
 @@ -1343,6 +1366,8 @@ static void qdev_e1000_reset(DeviceState *dev)
  
  static Property e1000_properties[] = {
  DEFINE_NIC_PROPERTIES(E1000State, conf),
 +DEFINE_PROP_BIT(x-qemu_13_compat, E1000State,
 +compat_flags, E1000_FLAG_QEMU_13, false),
  DEFINE_PROP_END_OF_LIST(),

Would it make more sense to simply call this auto-negotiation?

Regards,

Anthony Liguori

  };
  
 diff --git a/hw/pc_piix.c b/hw/pc_piix.c
 index ba09714..89a3f1f 100644
 --- a/hw/pc_piix.c
 +++ b/hw/pc_piix.c
 @@ -537,6 +537,10 @@ static QEMUMachine pc_machine_v0_13 = {
  .driver   = vmware-svga,
  .property = rombar,
  .value= stringify(0),
 +},{
 +.driver   = e1000,
 +.property = x-qemu_13_compat,
 +.value= on,
  },
  { /* end of list */ }
  },
 -- 
 MST

Re: [Qemu-devel] [PATCH for-1.4] e1000: unbreak the guest network migration to 1.3

2013-02-14 Thread Michael S. Tsirkin 
On Thu, Feb 14, 2013 at 01:50:04PM -0600, Anthony Liguori wrote:
 Michael S. Tsirkin m...@redhat.com writes:
 
  QEMU 1.3 does not emulate the link auto negotiation, so if migrate to a
  1.3 machine during link auto negotiation, the guest link will be set to 
  down.
  Fix this by just disabling auto negotiation for 1.3.
 
  Signed-off-by: Michael S. Tsirkin m...@redhat.com
  ---
   hw/e1000.c   | 25 +
   hw/pc_piix.c |  4 
   2 files changed, 29 insertions(+)
 
  diff --git a/hw/e1000.c b/hw/e1000.c
  index d6fe815..263f2f4 100644
  --- a/hw/e1000.c
  +++ b/hw/e1000.c
  @@ -131,6 +131,11 @@ typedef struct E1000State_st {
   } eecd_state;
   
   QEMUTimer *autoneg_timer;
  +
  +/* Enabled compatibility for migration to/from rhel6.3.0 and older */
 
 I presume you mean QEMU 1.3...

Yack.

  +#define E1000_FLAG_QEMU_13_BIT 0
  +#define E1000_FLAG_QEMU_13 (1  E1000_FLAG_QEMU_13_BIT)
  +uint32_t compat_flags;
   } E1000State;
   
   #definedefreg(x)   x = (E1000_##x2)
  @@ -165,6 +170,14 @@ e1000_link_up(E1000State *s)
   static void
   set_phy_ctrl(E1000State *s, int index, uint16_t val)
   {
  +/*
  + * QEMU 1.3 does not support link auto-negotiation emulation, so if we
  + * migrate during auto negotiation, after migration the link will be
  + * down.
  + */
  +if (s-compat_flags  E1000_FLAG_QEMU_13) {
  +return;
  +}
   if ((val  MII_CR_AUTO_NEG_EN)  (val  MII_CR_RESTART_AUTO_NEG)) {
   e1000_link_down(s);
   s-phy_reg[PHY_STATUS] = ~MII_SR_AUTONEG_COMPLETE;
  @@ -1120,6 +1133,11 @@ static void e1000_pre_save(void *opaque)
   {
   E1000State *s = opaque;
   NetClientState *nc = qemu_get_queue(s-nic);
  +
  +if (s-compat_flags  E1000_FLAG_QEMU_13) {
  +return;
  +}
  +
   /*
* If link is down and auto-negotiation is ongoing, complete
* auto-negotiation immediately.  This allows is to look at
  @@ -1141,6 +1159,11 @@ static int e1000_post_load(void *opaque, int 
  version_id)
* to link status bit in mac_reg[STATUS].
* Alternatively, restart link negotiation if it was in progress. */
   nc-link_down = (s-mac_reg[STATUS]  E1000_STATUS_LU) == 0;
  +
  +if (s-compat_flags  E1000_FLAG_QEMU_13) {
  +return 0;
  +}
  +
   if (s-phy_reg[PHY_CTRL]  MII_CR_AUTO_NEG_EN 
   s-phy_reg[PHY_CTRL]  MII_CR_RESTART_AUTO_NEG 
   !(s-phy_reg[PHY_STATUS]  MII_SR_AUTONEG_COMPLETE)) {
  @@ -1343,6 +1366,8 @@ static void qdev_e1000_reset(DeviceState *dev)
   
   static Property e1000_properties[] = {
   DEFINE_NIC_PROPERTIES(E1000State, conf),
  +DEFINE_PROP_BIT(x-qemu_13_compat, E1000State,
  +compat_flags, E1000_FLAG_QEMU_13, false),
   DEFINE_PROP_END_OF_LIST(),
 
 Would it make more sense to simply call this auto-negotiation?

Okay.

 Regards,
 
 Anthony Liguori
 
   };
   
  diff --git a/hw/pc_piix.c b/hw/pc_piix.c
  index ba09714..89a3f1f 100644
  --- a/hw/pc_piix.c
  +++ b/hw/pc_piix.c
  @@ -537,6 +537,10 @@ static QEMUMachine pc_machine_v0_13 = {
   .driver   = vmware-svga,
   .property = rombar,
   .value= stringify(0),
  +},{
  +.driver   = e1000,
  +.property = x-qemu_13_compat,
  +.value= on,
   },
   { /* end of list */ }
   },
  -- 
  MST