[libvirt] [PATCH 0/3] Un-break s390 default network model and clean up obsolete
This series fixes the default network model for s390 that was broken by the recent addition of post parse close callbacks and cleans up code obsoleted by that addition. Peter Krempa (2): qemu: Clean up network device CLI generator qemu: Remove now obsolete assignment of default netwokr card model for s390 hosts Viktor Mihajlovski (1): qemu: Use correct default model on s390 src/qemu/qemu_command.c | 24 +++++++----------------- src/qemu/qemu_domain.c | 12 +++++++++--- 2 files changed, 16 insertions(+), 20 deletions(-) -- 1.8.1.5
From: Viktor Mihajlovski <mihajlov@linux.vnet.ibm.com> Commit a68d6726679323823ee5be47f0144e9ccffa0757 breaks networking on s390 as it changes the default network card model. --- I assigned the authorship to Viktor as I just took his suggested patch from the mail thread regarding the close callback. src/qemu/qemu_domain.c | 12 +++++++++--- 1 file changed, 9 insertions(+), 3 deletions(-) diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c index f431aec..9dedd4a 100644 --- a/src/qemu/qemu_domain.c +++ b/src/qemu/qemu_domain.c @@ -678,7 +678,7 @@ qemuDomainDefPostParse(virDomainDefPtr def, static int qemuDomainDeviceDefPostParse(virDomainDeviceDefPtr dev, - virDomainDefPtr def ATTRIBUTE_UNUSED, + virDomainDefPtr def, virCapsPtr caps ATTRIBUTE_UNUSED, void *opaque) { @@ -688,9 +688,15 @@ qemuDomainDeviceDefPostParse(virDomainDeviceDefPtr dev, if (dev->type == VIR_DOMAIN_DEVICE_NET && dev->data.net->type != VIR_DOMAIN_NET_TYPE_HOSTDEV) { + const char *model = NULL; + if (def->os.arch == VIR_ARCH_S390 || + def->os.arch == VIR_ARCH_S390X) + model = "virtio"; + else + model = "rtl8139"; if (!dev->data.net->model && - !(dev->data.net->model = strdup("rtl8139"))) - goto no_memory; + !(dev->data.net->model = strdup(model))) + goto no_memory; } /* set default disk types and drivers */ -- 1.8.1.5
On 04/05/2013 12:12 PM, Peter Krempa wrote:
@@ -688,9 +688,15 @@ qemuDomainDeviceDefPostParse(virDomainDeviceDefPtr dev,
if (dev->type == VIR_DOMAIN_DEVICE_NET && dev->data.net->type != VIR_DOMAIN_NET_TYPE_HOSTDEV) { + const char *model = NULL; Now that I see what I wrote: the NULL assignment above is useless. I assume you will need a third pair of eyes?
-- Mit freundlichen Grüßen/Kind Regards Viktor Mihajlovski IBM Deutschland Research & Development GmbH Vorsitzender des Aufsichtsrats: Martina Köderitz Geschäftsführung: Dirk Wittkopp Sitz der Gesellschaft: Böblingen Registergericht: Amtsgericht Stuttgart, HRB 243294
On 04/05/13 13:02, Viktor Mihajlovski wrote:
On 04/05/2013 12:12 PM, Peter Krempa wrote:
@@ -688,9 +688,15 @@ qemuDomainDeviceDefPostParse(virDomainDeviceDefPtr dev,
if (dev->type == VIR_DOMAIN_DEVICE_NET && dev->data.net->type != VIR_DOMAIN_NET_TYPE_HOSTDEV) { + const char *model = NULL; Now that I see what I wrote: the NULL assignment above is useless. I assume you will need a third pair of eyes?
Oh, indeed :). My morning coffee hasn't kicked in at that time. We can even drop the model variable and duplicate the strdups to avoid the intermediate variable. V2 commencing. Peter
From: Viktor Mihajlovski <mihajlov@linux.vnet.ibm.com> Commit a68d6726679323823ee5be47f0144e9ccffa0757 breaks networking on s390 as it changes the default network card model. --- src/qemu/qemu_domain.c | 16 +++++++++++----- 1 file changed, 11 insertions(+), 5 deletions(-) diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c index f431aec..6ed2b40 100644 --- a/src/qemu/qemu_domain.c +++ b/src/qemu/qemu_domain.c @@ -678,7 +678,7 @@ qemuDomainDefPostParse(virDomainDefPtr def, static int qemuDomainDeviceDefPostParse(virDomainDeviceDefPtr dev, - virDomainDefPtr def ATTRIBUTE_UNUSED, + virDomainDefPtr def, virCapsPtr caps ATTRIBUTE_UNUSED, void *opaque) { @@ -687,10 +687,16 @@ qemuDomainDeviceDefPostParse(virDomainDeviceDefPtr dev, virQEMUDriverConfigPtr cfg = NULL; if (dev->type == VIR_DOMAIN_DEVICE_NET && - dev->data.net->type != VIR_DOMAIN_NET_TYPE_HOSTDEV) { - if (!dev->data.net->model && - !(dev->data.net->model = strdup("rtl8139"))) - goto no_memory; + dev->data.net->type != VIR_DOMAIN_NET_TYPE_HOSTDEV && + !dev->data.net->model) { + if (def->os.arch == VIR_ARCH_S390 || + def->os.arch == VIR_ARCH_S390X) + dev->data.net->model = strdup("virtio"); + else + dev->data.net->model = strdup("rtl8139"); + + if (!dev->data.net->model) + goto no_memory; } /* set default disk types and drivers */ -- 1.8.1.5
On 04/05/2013 03:16 PM, Peter Krempa wrote:
From: Viktor Mihajlovski <mihajlov@linux.vnet.ibm.com>
Commit a68d6726679323823ee5be47f0144e9ccffa0757 breaks networking on s390 as it changes the default network card model. --- src/qemu/qemu_domain.c | 16 +++++++++++----- 1 file changed, 11 insertions(+), 5 deletions(-)
+1 -- Mit freundlichen Grüßen/Kind Regards Viktor Mihajlovski IBM Deutschland Research & Development GmbH Vorsitzender des Aufsichtsrats: Martina Köderitz Geschäftsführung: Dirk Wittkopp Sitz der Gesellschaft: Böblingen Registergericht: Amtsgericht Stuttgart, HRB 243294
With the default model assigned in the parse callback, this code is now obsolete. --- src/qemu/qemu_command.c | 19 +++++++------------ 1 file changed, 7 insertions(+), 12 deletions(-) diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index 493e5f8..8a76fba 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -3664,27 +3664,22 @@ qemuBuildNicDevStr(virDomainNetDefPtr net, virQEMUCapsPtr qemuCaps) { virBuffer buf = VIR_BUFFER_INITIALIZER; - const char *nic; + const char *nic = net->model; bool usingVirtio = false; char macaddr[VIR_MAC_STRING_BUFLEN]; - if (!net->model) { - nic = "rtl8139"; - } else if (STREQ(net->model, "virtio")) { - if (net->info.type == VIR_DOMAIN_DEVICE_ADDRESS_TYPE_CCW) { + if (STREQ(net->model, "virtio")) { + if (net->info.type == VIR_DOMAIN_DEVICE_ADDRESS_TYPE_CCW) nic = "virtio-net-ccw"; - } else if (net->info.type == - VIR_DOMAIN_DEVICE_ADDRESS_TYPE_VIRTIO_S390) { + else if (net->info.type == VIR_DOMAIN_DEVICE_ADDRESS_TYPE_VIRTIO_S390) nic = "virtio-net-s390"; - } else { + else nic = "virtio-net-pci"; - } + usingVirtio = true; - } else { - nic = net->model; } - virBufferAdd(&buf, nic, strlen(nic)); + virBufferAdd(&buf, nic, -1); if (usingVirtio && net->driver.virtio.txmode) { if (virQEMUCapsGet(qemuCaps, QEMU_CAPS_VIRTIO_TX_ALG)) { virBufferAddLit(&buf, ",tx="); -- 1.8.1.5
On 04/05/2013 12:12 PM, Peter Krempa wrote:
With the default model assigned in the parse callback, this code is now obsolete. --- src/qemu/qemu_command.c | 19 +++++++------------ 1 file changed, 7 insertions(+), 12 deletions(-)
[Unauthoritative] ACK. -- Mit freundlichen Grüßen/Kind Regards Viktor Mihajlovski IBM Deutschland Research & Development GmbH Vorsitzender des Aufsichtsrats: Martina Köderitz Geschäftsführung: Dirk Wittkopp Sitz der Gesellschaft: Böblingen Registergericht: Amtsgericht Stuttgart, HRB 243294
This effectively reverts commit 539d73dbf64cb35558ffd3992f82e0b482cd0e70 as the changes aren't needed after introduction of the XML post parse callbacks. --- src/qemu/qemu_command.c | 5 ----- 1 file changed, 5 deletions(-) diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index 8a76fba..dd3e5ca 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -919,11 +919,6 @@ qemuDomainPrimeS390VirtioDevices(virDomainDefPtr def, } for (i = 0; i < def->nnets ; i++) { - if ((def->os.arch == VIR_ARCH_S390 || - def->os.arch == VIR_ARCH_S390X) && - def->nets[i]->model == NULL) { - def->nets[i]->model = strdup("virtio"); - } if (STREQ(def->nets[i]->model,"virtio") && def->nets[i]->info.type == VIR_DOMAIN_DEVICE_ADDRESS_TYPE_NONE) { def->nets[i]->info.type = type; -- 1.8.1.5
On 04/05/2013 12:12 PM, Peter Krempa wrote:
This effectively reverts commit 539d73dbf64cb35558ffd3992f82e0b482cd0e70 as the changes aren't needed after introduction of the XML post parse callbacks. --- src/qemu/qemu_command.c | 5 ----- 1 file changed, 5 deletions(-)
diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index 8a76fba..dd3e5ca 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -919,11 +919,6 @@ qemuDomainPrimeS390VirtioDevices(virDomainDefPtr def, }
for (i = 0; i < def->nnets ; i++) { - if ((def->os.arch == VIR_ARCH_S390 || - def->os.arch == VIR_ARCH_S390X) && - def->nets[i]->model == NULL) { - def->nets[i]->model = strdup("virtio"); - } if (STREQ(def->nets[i]->model,"virtio") && def->nets[i]->info.type == VIR_DOMAIN_DEVICE_ADDRESS_TYPE_NONE) { def->nets[i]->info.type = type;
[Unauthoritative] ACK -- Mit freundlichen Grüßen/Kind Regards Viktor Mihajlovski IBM Deutschland Research & Development GmbH Vorsitzender des Aufsichtsrats: Martina Köderitz Geschäftsführung: Dirk Wittkopp Sitz der Gesellschaft: Böblingen Registergericht: Amtsgericht Stuttgart, HRB 243294
Oops: typo in the subject line s/netwokr/network/ -- Mit freundlichen Grüßen/Kind Regards Viktor Mihajlovski IBM Deutschland Research & Development GmbH Vorsitzender des Aufsichtsrats: Martina Köderitz Geschäftsführung: Dirk Wittkopp Sitz der Gesellschaft: Böblingen Registergericht: Amtsgericht Stuttgart, HRB 243294
On 04/05/2013 12:12 PM, Peter Krempa wrote:
This series fixes the default network model for s390 that was broken by the recent addition of post parse close callbacks and cleans up code obsoleted by that addition.
Peter Krempa (2): qemu: Clean up network device CLI generator qemu: Remove now obsolete assignment of default netwokr card model for s390 hosts
Viktor Mihajlovski (1): qemu: Use correct default model on s390
src/qemu/qemu_command.c | 24 +++++++----------------- src/qemu/qemu_domain.c | 12 +++++++++--- 2 files changed, 16 insertions(+), 20 deletions(-)
ACK series Jan
On 04/09/13 15:41, Ján Tomko wrote:
On 04/05/2013 12:12 PM, Peter Krempa wrote:
This series fixes the default network model for s390 that was broken by the recent addition of post parse close callbacks and cleans up code obsoleted by that addition.
Peter Krempa (2): qemu: Clean up network device CLI generator qemu: Remove now obsolete assignment of default netwokr card model for s390 hosts
Viktor Mihajlovski (1): qemu: Use correct default model on s390
src/qemu/qemu_command.c | 24 +++++++----------------- src/qemu/qemu_domain.c | 12 +++++++++--- 2 files changed, 16 insertions(+), 20 deletions(-)
ACK series
Thanks for the reviews. The series is pushed now. Peter
participants (3)
-
Ján Tomko -
Peter Krempa -
Viktor Mihajlovski