Merge ~paelzer/ubuntu/+source/libvirt:post-disco-sru-set-cosmic into ubuntu/+source/libvirt:ubuntu/cosmic-devel

Proposed by Christian Ehrhardt 
Status: Merged
Approved by: Andreas Hasenack
Approved revision: 30ace54e149a8b2d917f97e6303054dae0bcb4e7
Merge reported by: Christian Ehrhardt 
Merged at revision: d20c8a61d4aa9c2063790dc39967412862961457
Proposed branch: ~paelzer/ubuntu/+source/libvirt:post-disco-sru-set-cosmic
Merge into: ubuntu/+source/libvirt:ubuntu/cosmic-devel
Diff against target: 426 lines (+380/-0)
7 files modified
debian/changelog (+9/-0)
debian/patches/series (+5/-0)
debian/patches/ubuntu/lp-1771662-1-util-fixing-wrong-assumption-that-PF-has-to-have-net.patch (+62/-0)
debian/patches/ubuntu/lp-1771662-2-util-Code-simplification.patch (+104/-0)
debian/patches/ubuntu/lp-1771662-3-util-Fix-for-NULL-dereference.patch (+74/-0)
debian/patches/ubuntu/lp-1771662-4-util-Fixing-invalid-error-checking-from-virPCIGetNet.patch (+35/-0)
debian/patches/ubuntu/lp-1811198-utils-Remove-arbitrary-limit-on-socket_id-core_id.patch (+91/-0)
Reviewer Review Type Date Requested Status
Andreas Hasenack Approve
Canonical Server Pending
git-ubuntu developers Pending
Review via email: mp+362544@code.launchpad.net
To post a comment you must log in.
Revision history for this message
Christian Ehrhardt  (paelzer) wrote :
Revision history for this message
Christian Ehrhardt  (paelzer) wrote :

dep8 tests are only hitting known flaky tests, so they are ok

Also the automated virt regression tests ran fine on all architectures.

Remaining: I'm waiting for the bug reporters to confirm test-ability and the MP review here

Revision history for this message
Andreas Hasenack (ahasenack) wrote :

Just an incorrect bug number in one of the dep3 headers.

I also wondered if lp-1771662-4-util-Fixing-invalid-error-checking-from-virPCIGetNet.patch warranted a "backport" note, since it's just a whitespace change on a single line. I would have fixed the whitespace and at most added a comment to the patch header, but that's just a personal opinion.

+1 with the bug number fixed.

review: Approve
Revision history for this message
Christian Ehrhardt  (paelzer) wrote :

Thanks, I fixed the bug reference.

For the backport note, I strictly follow "if it applies as-is then upstream, otherwise backport".
That way I don't have to make decisions where "real" backports start because any other line we draw can be just as wrong from other peoples POV.

There was an error fetching revisions from git servers. Please try again in a few minutes. If the problem persists, contact Launchpad support.

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
diff --git a/debian/changelog b/debian/changelog
index 0cf4887..9cafb4c 100644
--- a/debian/changelog
+++ b/debian/changelog
@@ -1,3 +1,12 @@
1libvirt (4.6.0-2ubuntu3.3) cosmic; urgency=medium
2
3 * d/p/ubuntu/lp-1811198-utils-Remove-arbitrary-limit-on-socket_id-core_id
4 .patch: fix arm servers with high core_id (LP: #1811198)
5 * d/p/ubuntu/lp-1771662-*: fix assumption that all VFs have PFs assigned
6 (LP: #1771662)
7
8 -- Christian Ehrhardt <christian.ehrhardt@canonical.com> Thu, 31 Jan 2019 12:29:37 +0100
9
1libvirt (4.6.0-2ubuntu3.2) cosmic; urgency=medium10libvirt (4.6.0-2ubuntu3.2) cosmic; urgency=medium
211
3 * d/p/ubuntu/lp1787405-0008-qemu-mdev-Use-vfio-pci-display-property-only12 * d/p/ubuntu/lp1787405-0008-qemu-mdev-Use-vfio-pci-display-property-only
diff --git a/debian/patches/series b/debian/patches/series
index 69ff83a..5e60dd2 100644
--- a/debian/patches/series
+++ b/debian/patches/series
@@ -70,3 +70,8 @@ ubuntu/lp1787405-0005-qemu-Extract-MDEV-VFIO-PCI-validation-code-into-a-se.patch
70ubuntu/lp1787405-0006-conf-Move-VFIO-AP-validation-from-post-parse-to-QEMU.patch70ubuntu/lp1787405-0006-conf-Move-VFIO-AP-validation-from-post-parse-to-QEMU.patch
71ubuntu/lp1787405-0007-virt-aa-helper-mdevs-need-vfio.patch71ubuntu/lp1787405-0007-virt-aa-helper-mdevs-need-vfio.patch
72ubuntu/lp1787405-0008-qemu-mdev-Use-vfio-pci-display-property-only-with-vf.patch72ubuntu/lp1787405-0008-qemu-mdev-Use-vfio-pci-display-property-only-with-vf.patch
73ubuntu/lp-1811198-utils-Remove-arbitrary-limit-on-socket_id-core_id.patch
74ubuntu/lp-1771662-1-util-fixing-wrong-assumption-that-PF-has-to-have-net.patch
75ubuntu/lp-1771662-2-util-Code-simplification.patch
76ubuntu/lp-1771662-3-util-Fix-for-NULL-dereference.patch
77ubuntu/lp-1771662-4-util-Fixing-invalid-error-checking-from-virPCIGetNet.patch
diff --git a/debian/patches/ubuntu/lp-1771662-1-util-fixing-wrong-assumption-that-PF-has-to-have-net.patch b/debian/patches/ubuntu/lp-1771662-1-util-fixing-wrong-assumption-that-PF-has-to-have-net.patch
73new file mode 10064478new file mode 100644
index 0000000..a7944e1
--- /dev/null
+++ b/debian/patches/ubuntu/lp-1771662-1-util-fixing-wrong-assumption-that-PF-has-to-have-net.patch
@@ -0,0 +1,62 @@
1From 6452e2f5e1837bd753ee465e6607ed3c4f62b815 Mon Sep 17 00:00:00 2001
2From: Radoslaw Biernacki <radoslaw.biernacki@linaro.org>
3Date: Tue, 22 Jan 2019 12:26:12 -0700
4Subject: [PATCH 1/4] util: fixing wrong assumption that PF has to have netdev
5 assigned
6
7libvirt wrongly assumes that VF netdev has to have the
8netdev assigned to PF. There is no such requirement in SRIOV standard.
9This patch change the virNetDevSwitchdevFeature() function to deal
10with SRIOV devices which does not have netdev on PF. Also corrects
11one comment about PF netdev assumption.
12
13One example of such devices is ThunderX VNIC.
14By applying this change, VF device is used for virNetlinkCommand() as
15it is the only netdev assigned to VNIC.
16
17Signed-off-by: Radoslaw Biernacki <radoslaw.biernacki@linaro.org>
18Signed-off-by: dann frazier <dann.frazier@canonical.com>
19Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
20
21Origin: upstream, https://libvirt.org/git/?p=libvirt.git;a=commit;h=6452e2f5e1837bd753ee465e6607ed3c4f62b815
22Bug-Ubuntu: https://bugs.launchpad.net/bugs/1771662
23Last-Update: 2019-01-31
24
25---
26 src/util/virnetdev.c | 13 ++++++++-----
27 1 file changed, 8 insertions(+), 5 deletions(-)
28
29diff --git a/src/util/virnetdev.c b/src/util/virnetdev.c
30index 2111b3ada9..d3e69b6f60 100644
31--- a/src/util/virnetdev.c
32+++ b/src/util/virnetdev.c
33@@ -1355,9 +1355,8 @@ virNetDevGetPhysicalFunction(const char *ifname, char **pfname)
34 }
35
36 if (!*pfname) {
37- /* this shouldn't be possible. A VF can't exist unless its
38- * PF device is bound to a network driver
39- */
40+ /* The SRIOV standard does not require VF netdevs to have
41+ * the netdev assigned to a PF. */
42 virReportError(VIR_ERR_INTERNAL_ERROR,
43 _("The PF device for VF %s has no network device name"),
44 ifname);
45@@ -3178,8 +3177,12 @@ virNetDevSwitchdevFeature(const char *ifname,
46 if ((is_vf = virNetDevIsVirtualFunction(ifname)) < 0)
47 return ret;
48
49- if (is_vf == 1 && virNetDevGetPhysicalFunction(ifname, &pfname) < 0)
50- goto cleanup;
51+ if (is_vf == 1) {
52+ /* Ignore error if PF does not have netdev assigned.
53+ * In that case pfname == NULL. */
54+ if (virNetDevGetPhysicalFunction(ifname, &pfname) < 0)
55+ virResetLastError();
56+ }
57
58 pci_device_ptr = pfname ? virNetDevGetPCIDevice(pfname) :
59 virNetDevGetPCIDevice(ifname);
60--
612.17.1
62
diff --git a/debian/patches/ubuntu/lp-1771662-2-util-Code-simplification.patch b/debian/patches/ubuntu/lp-1771662-2-util-Code-simplification.patch
0new file mode 10064463new file mode 100644
index 0000000..bcb8a6f
--- /dev/null
+++ b/debian/patches/ubuntu/lp-1771662-2-util-Code-simplification.patch
@@ -0,0 +1,104 @@
1From 10bca495e040ef834760a244a31f8b87391c2378 Mon Sep 17 00:00:00 2001
2From: Radoslaw Biernacki <radoslaw.biernacki@linaro.org>
3Date: Tue, 22 Jan 2019 12:26:13 -0700
4Subject: [PATCH 2/4] util: Code simplification
5
6Removing redundant sections of the code
7
8Signed-off-by: Radoslaw Biernacki <radoslaw.biernacki@linaro.org>
9Signed-off-by: dann frazier <dann.frazier@canonical.com>
10Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
11
12Origin: upstream, https://libvirt.org/git/?p=libvirt.git;a=commit;h=10bca495e040ef834760a244a31f8b87391c2378
13Bug-Ubuntu: https://bugs.launchpad.net/bugs/1771662
14Last-Update: 2019-01-31
15
16---
17 src/util/virnetdev.c | 33 ++++++---------------------------
18 1 file changed, 6 insertions(+), 27 deletions(-)
19
20diff --git a/src/util/virnetdev.c b/src/util/virnetdev.c
21index d3e69b6f60..92ef008ca1 100644
22--- a/src/util/virnetdev.c
23+++ b/src/util/virnetdev.c
24@@ -1443,29 +1443,20 @@ int
25 virNetDevGetVirtualFunctionInfo(const char *vfname, char **pfname,
26 int *vf)
27 {
28- char *pf_sysfs_path = NULL, *vf_sysfs_path = NULL;
29 int ret = -1;
30
31 *pfname = NULL;
32
33 if (virNetDevGetPhysicalFunction(vfname, pfname) < 0)
34- return ret;
35-
36- if (virNetDevSysfsFile(&pf_sysfs_path, *pfname, "device") < 0)
37- goto cleanup;
38+ return -1;
39
40- if (virNetDevSysfsFile(&vf_sysfs_path, vfname, "device") < 0)
41+ if (virNetDevGetVirtualFunctionIndex(*pfname, vfname, vf) < 0)
42 goto cleanup;
43
44- ret = virPCIGetVirtualFunctionIndex(pf_sysfs_path, vf_sysfs_path, vf);
45-
46+ ret = 0;
47 cleanup:
48 if (ret < 0)
49 VIR_FREE(*pfname);
50-
51- VIR_FREE(vf_sysfs_path);
52- VIR_FREE(pf_sysfs_path);
53-
54 return ret;
55 }
56
57@@ -1861,13 +1852,9 @@ virNetDevSaveNetConfig(const char *linkdev, int vf,
58 * it to PF + VFname
59 */
60
61- if (virNetDevGetPhysicalFunction(linkdev, &pfDevOrig) < 0)
62+ if (virNetDevGetVirtualFunctionInfo(linkdev, &pfDevOrig, &vf) < 0)
63 goto cleanup;
64-
65 pfDevName = pfDevOrig;
66-
67- if (virNetDevGetVirtualFunctionIndex(pfDevName, linkdev, &vf) < 0)
68- goto cleanup;
69 }
70
71 if (pfDevName) {
72@@ -2019,13 +2006,9 @@ virNetDevReadNetConfig(const char *linkdev, int vf,
73 * it to PF + VFname
74 */
75
76- if (virNetDevGetPhysicalFunction(linkdev, &pfDevOrig) < 0)
77+ if (virNetDevGetVirtualFunctionInfo(linkdev, &pfDevOrig, &vf) < 0)
78 goto cleanup;
79-
80 pfDevName = pfDevOrig;
81-
82- if (virNetDevGetVirtualFunctionIndex(pfDevName, linkdev, &vf) < 0)
83- goto cleanup;
84 }
85
86 /* if there is a PF, it's now in pfDevName, and linkdev is either
87@@ -2224,13 +2207,9 @@ virNetDevSetNetConfig(const char *linkdev, int vf,
88 * it to PF + VFname
89 */
90
91- if (virNetDevGetPhysicalFunction(linkdev, &pfDevOrig) < 0)
92+ if (virNetDevGetVirtualFunctionInfo(linkdev, &pfDevOrig, &vf))
93 goto cleanup;
94-
95 pfDevName = pfDevOrig;
96-
97- if (virNetDevGetVirtualFunctionIndex(pfDevName, linkdev, &vf) < 0)
98- goto cleanup;
99 }
100
101
102--
1032.17.1
104
diff --git a/debian/patches/ubuntu/lp-1771662-3-util-Fix-for-NULL-dereference.patch b/debian/patches/ubuntu/lp-1771662-3-util-Fix-for-NULL-dereference.patch
0new file mode 100644105new file mode 100644
index 0000000..4a8cd91
--- /dev/null
+++ b/debian/patches/ubuntu/lp-1771662-3-util-Fix-for-NULL-dereference.patch
@@ -0,0 +1,74 @@
1From 8fac64db5e7941efb820626f0043f5e0a31c79ee Mon Sep 17 00:00:00 2001
2From: Radoslaw Biernacki <radoslaw.biernacki@linaro.org>
3Date: Tue, 22 Jan 2019 12:26:14 -0700
4Subject: [PATCH 3/4] util: Fix for NULL dereference
5
6The device xml parser code does not set "model" while parsing the
7following XML:
8
9 <interface type='hostdev'>
10 <source>
11 <address type='pci' domain='0x0002' bus='0x01' slot='0x00' function='0x2'/>
12 </source>
13 </interface>
14
15The net->model can be NULL and therefore must be compared using
16STREQ_NULLABLE instead of plain STREQ.
17
18Fixes: ac47e4a6225 (qemu: replace "def->nets[i]" with "net" and "def->sounds[i]" with "sound")
19Fixes: c7fc151eec7 (qemu: assign virtio devices to PCIe slot when appropriate)
20Signed-off-by: Radoslaw Biernacki <radoslaw.biernacki@linaro.org>
21Signed-off-by: dann frazier <dann.frazier@canonical.com>
22Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
23
24Author: Christian Ehrhardt <christian.ehrhardt@canonical.com>
25Original-Author: Radoslaw Biernacki <radoslaw.biernacki@linaro.org>
26Origin: backport, https://libvirt.org/git/?p=libvirt.git;a=commit;h=8fac64db5e7941efb820626f0043f5e0a31c79ee
27Bug-Ubuntu: https://bugs.launchpad.net/bugs/1771662
28Last-Update: 2019-01-31
29
30---
31 src/qemu/qemu_domain_address.c | 13 +++++--------
32 1 file changed, 5 insertions(+), 8 deletions(-)
33
34--- a/src/qemu/qemu_domain_address.c
35+++ b/src/qemu/qemu_domain_address.c
36@@ -232,10 +232,8 @@ qemuDomainAssignSpaprVIOAddresses(virDom
37 for (i = 0; i < def->nnets; i++) {
38 virDomainNetDefPtr net = def->nets[i];
39
40- if (net->model &&
41- STREQ(net->model, "spapr-vlan")) {
42+ if (STREQ_NULLABLE(net->model, "spapr-vlan"))
43 net->info.type = VIR_DOMAIN_DEVICE_ADDRESS_TYPE_SPAPRVIO;
44- }
45
46 if (qemuDomainAssignSpaprVIOAddress(def, &net->info, VIO_ADDR_NET) < 0)
47 goto cleanup;
48@@ -323,7 +321,7 @@ qemuDomainPrimeVirtioDeviceAddresses(vir
49 for (i = 0; i < def->nnets; i++) {
50 virDomainNetDefPtr net = def->nets[i];
51
52- if (STREQ(net->model, "virtio") &&
53+ if (STREQ_NULLABLE(net->model, "virtio") &&
54 net->info.type == VIR_DOMAIN_DEVICE_ADDRESS_TYPE_NONE) {
55 net->info.type = type;
56 }
57@@ -604,14 +602,14 @@ qemuDomainDeviceCalculatePCIConnectFlags
58 * addresses for other hostdev devices.
59 */
60 if (net->type == VIR_DOMAIN_NET_TYPE_HOSTDEV ||
61- STREQ(net->model, "usb-net")) {
62+ STREQ_NULLABLE(net->model, "usb-net")) {
63 return 0;
64 }
65
66- if (STREQ(net->model, "virtio"))
67+ if (STREQ_NULLABLE(net->model, "virtio"))
68 return virtioFlags;
69
70- if (STREQ(net->model, "e1000e"))
71+ if (STREQ_NULLABLE(net->model, "e1000e"))
72 return pcieFlags;
73
74 return pciFlags;
diff --git a/debian/patches/ubuntu/lp-1771662-4-util-Fixing-invalid-error-checking-from-virPCIGetNet.patch b/debian/patches/ubuntu/lp-1771662-4-util-Fixing-invalid-error-checking-from-virPCIGetNet.patch
0new file mode 10064475new file mode 100644
index 0000000..6b002c7
--- /dev/null
+++ b/debian/patches/ubuntu/lp-1771662-4-util-Fixing-invalid-error-checking-from-virPCIGetNet.patch
@@ -0,0 +1,35 @@
1From 04983c3c6a821f67994b1c65d4d6175f3ac49d69 Mon Sep 17 00:00:00 2001
2From: Radoslaw Biernacki <radoslaw.biernacki@linaro.org>
3Date: Tue, 22 Jan 2019 12:26:15 -0700
4Subject: [PATCH 4/4] util: Fixing invalid error checking from
5 virPCIGetNetname()
6
7The @linkdev is In/Out function parameter as second order
8reference pointer so requires first order dereference for
9checking NULL which can be the result of virPCIGetNetName().
10
11Fixes: d6ee56d7237 (util: change virPCIGetNetName() to not return error if device has no net name)
12Signed-off-by: Radoslaw Biernacki <radoslaw.biernacki@linaro.org>
13Signed-off-by: dann frazier <dann.frazier@canonical.com>
14
15Author: Christian Ehrhardt <christian.ehrhardt@canonical.com>
16Original-Author: Radoslaw Biernacki <radoslaw.biernacki@linaro.org>
17Origin: backport, https://libvirt.org/git/?p=libvirt.git;a=commit;h=04983c3c6a821f67994b1c65d4d6175f3ac49d69
18Bug-Ubuntu: https://bugs.launchpad.net/bugs/1771662
19Last-Update: 2019-01-31
20
21---
22 src/util/virhostdev.c | 2 +-
23 1 file changed, 1 insertion(+), 1 deletion(-)
24
25--- a/src/util/virhostdev.c
26+++ b/src/util/virhostdev.c
27@@ -319,7 +319,7 @@ virHostdevNetDevice(virDomainHostdevDefP
28 if (virPCIGetNetName(sysfs_path, 0, NULL, linkdev) < 0)
29 return -1;
30
31- if (!linkdev) {
32+ if (!(*linkdev)) {
33 virReportError(VIR_ERR_INTERNAL_ERROR,
34 _("The device at %s has no network device name"),
35 sysfs_path);
diff --git a/debian/patches/ubuntu/lp-1811198-utils-Remove-arbitrary-limit-on-socket_id-core_id.patch b/debian/patches/ubuntu/lp-1811198-utils-Remove-arbitrary-limit-on-socket_id-core_id.patch
0new file mode 10064436new file mode 100644
index 0000000..81ae0fd
--- /dev/null
+++ b/debian/patches/ubuntu/lp-1811198-utils-Remove-arbitrary-limit-on-socket_id-core_id.patch
@@ -0,0 +1,91 @@
1From ba35ac2ebbc7f94abc50ffbf1d681458e2406444 Mon Sep 17 00:00:00 2001
2From: Andrea Bolognani <abologna@redhat.com>
3Date: Fri, 3 Aug 2018 10:15:16 +0200
4Subject: [PATCH] utils: Remove arbitrary limit on socket_id/core_id
5
6While in most cases the values are going to be much
7smaller than our arbitrary 4096 limit, there is really
8no guarantee that would be the case: in fact, a few
9aarch64 servers have been spotted in the wild with
10core_id as high as 6216.
11
12Take advantage of virBitmap's ability to automatically
13alter its size at runtime to accomodate such values.
14
15Signed-off-by: Andrea Bolognani <abologna@redhat.com>
16
17Origin: upstream, https://libvirt.org/git/?p=libvirt.git;a=commit;h=ba35ac2ebbc7f94abc50ffbf1d681458e2406444
18Bug-Ubuntu: https://bugs.launchpad.net/bugs/1811198
19Last-Update: 2019-01-31
20
21---
22 src/util/virhostcpu.c | 23 ++++-------------------
23 1 file changed, 4 insertions(+), 19 deletions(-)
24
25diff --git a/src/util/virhostcpu.c b/src/util/virhostcpu.c
26index 6e79445abc..2337ad7d61 100644
27--- a/src/util/virhostcpu.c
28+++ b/src/util/virhostcpu.c
29@@ -293,9 +293,6 @@ virHostCPUParseNode(const char *node,
30 int *threads,
31 int *offline)
32 {
33- /* Biggest value we can expect to be used as either socket id
34- * or core id. Bitmaps will need to be sized accordingly */
35- const int ID_MAX = 4095;
36 int ret = -1;
37 int processors = 0;
38 DIR *cpudir = NULL;
39@@ -324,7 +321,7 @@ virHostCPUParseNode(const char *node,
40 goto cleanup;
41
42 /* enumerate sockets in the node */
43- if (!(sockets_map = virBitmapNew(ID_MAX + 1)))
44+ if (!(sockets_map = virBitmapNewEmpty()))
45 goto cleanup;
46
47 while ((direrr = virDirRead(cpudir, &cpudirent, node)) > 0) {
48@@ -343,14 +340,8 @@ virHostCPUParseNode(const char *node,
49
50 if (virHostCPUGetSocket(cpu, &sock) < 0)
51 goto cleanup;
52- if (sock > ID_MAX) {
53- virReportError(VIR_ERR_INTERNAL_ERROR,
54- _("Socket %d can't be handled (max socket is %d)"),
55- sock, ID_MAX);
56- goto cleanup;
57- }
58
59- if (virBitmapSetBit(sockets_map, sock) < 0)
60+ if (virBitmapSetBitExpand(sockets_map, sock) < 0)
61 goto cleanup;
62
63 if (sock > sock_max)
64@@ -367,7 +358,7 @@ virHostCPUParseNode(const char *node,
65 goto cleanup;
66
67 for (i = 0; i < sock_max; i++)
68- if (!(cores_maps[i] = virBitmapNew(ID_MAX + 1)))
69+ if (!(cores_maps[i] = virBitmapNewEmpty()))
70 goto cleanup;
71
72 /* Iterate over all CPUs in the node, in ascending order */
73@@ -411,14 +402,8 @@ virHostCPUParseNode(const char *node,
74 if (virHostCPUGetCore(cpu, &core) < 0)
75 goto cleanup;
76 }
77- if (core > ID_MAX) {
78- virReportError(VIR_ERR_INTERNAL_ERROR,
79- _("Core %d can't be handled (max core is %d)"),
80- core, ID_MAX);
81- goto cleanup;
82- }
83
84- if (virBitmapSetBit(cores_maps[sock], core) < 0)
85+ if (virBitmapSetBitExpand(cores_maps[sock], core) < 0)
86 goto cleanup;
87
88 if (!(siblings = virHostCPUCountThreadSiblings(cpu)))
89--
902.17.1
91

Subscribers

People subscribed via source and target branches