[libvirt] [PATCH] libxl: Fix devid init in libxlMakeNicList
This basically reverts commit ba64b97134a6129a48684f22f31be92c3b6eef96
libxl: Allow libxl to set NIC devid. However assigning devid's
before calling libxlMakeNic does not work as that is calling
libxl_device_nic_init which sets it back to -1.
Right now auto-assignment only works in the hotplug case. But even if
that would be fixed at some point (if that is possible at all), this
would add a weird dependency between Xen and libvirt versions.
The change here should accept any auto-assignment that makes it into
libxl_device_nic_init. My understanding is that a caller always is
allowed to make the devid choice itself. And assuming libxlMakeNicList
is only used on domain creation, a sequential numbering should be ok.
Signed-off-by: Stefan Bader stefan.ba...@canonical.com
---
src/libxl/libxl_conf.c |7 +++
1 file changed, 7 insertions(+)
diff --git a/src/libxl/libxl_conf.c b/src/libxl/libxl_conf.c
index 04d01af..4cefadf 100644
--- a/src/libxl/libxl_conf.c
+++ b/src/libxl/libxl_conf.c
@@ -918,6 +918,13 @@ libxlMakeNicList(virDomainDefPtr def, libxl_domain_config
*d_config)
for (i = 0; i nnics; i++) {
if (libxlMakeNic(def, l_nics[i], x_nics[i]))
goto error;
+/*
+ * The devid (at least right now) will not get initialized by
+ * libxl in the setup case but is required for starting the
+ * device-model.
+ */
+if (x_nics[i].devid 0)
+x_nics[i].devid = i;
}
d_config-nics = x_nics;
--
1.7.9.5
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] libxl: Fix devid init in libxlMakeNicList
On Wed, 2014-01-08 at 11:39 +0100, Stefan Bader wrote: This basically reverts commit ba64b97134a6129a48684f22f31be92c3b6eef96 libxl: Allow libxl to set NIC devid. However assigning devid's before calling libxlMakeNic does not work as that is calling libxl_device_nic_init which sets it back to -1. Right now auto-assignment only works in the hotplug case. But even if that would be fixed at some point (if that is possible at all), I think it should be -- any chance you could prepare a patch to libxl as well? -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] libxl: Fix devid init in libxlMakeNicList
On 08.01.2014 11:42, Ian Campbell wrote: On Wed, 2014-01-08 at 11:39 +0100, Stefan Bader wrote: This basically reverts commit ba64b97134a6129a48684f22f31be92c3b6eef96 libxl: Allow libxl to set NIC devid. However assigning devid's before calling libxlMakeNic does not work as that is calling libxl_device_nic_init which sets it back to -1. Right now auto-assignment only works in the hotplug case. But even if that would be fixed at some point (if that is possible at all), I think it should be -- any chance you could prepare a patch to libxl as well? Not sure that is that simple. At least not in libxl_device_nic_init. The way I understand this is done does not seem to allow a good solution. Creation looks to allow initializing NIC definitions in some array that is not connected to a specific domain, yet. I suppose what I could come up with is the kind of modification I was proposing in one of those emails last year (sounds like an awful long time :)). So at the point where the device-model is about to get started and the code already loops over the NIC definitions. That won't give the libvirt path default devid's but any other user that has not done yet. -Stefan signature.asc Description: OpenPGP digital signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] libxl: Fix devid init in libxlMakeNicList
On Wed, 2014-01-08 at 12:20 +0100, Stefan Bader wrote: On 08.01.2014 11:42, Ian Campbell wrote: On Wed, 2014-01-08 at 11:39 +0100, Stefan Bader wrote: This basically reverts commit ba64b97134a6129a48684f22f31be92c3b6eef96 libxl: Allow libxl to set NIC devid. However assigning devid's before calling libxlMakeNic does not work as that is calling libxl_device_nic_init which sets it back to -1. Right now auto-assignment only works in the hotplug case. But even if that would be fixed at some point (if that is possible at all), I think it should be -- any chance you could prepare a patch to libxl as well? Not sure that is that simple. At least not in libxl_device_nic_init. The way I understand this is done does not seem to allow a good solution. Creation looks to allow initializing NIC definitions in some array that is not connected to a specific domain, yet. Right, I think I previously concluded that for the domain create case this could be done in initiate_domain_create, with the hotplug case handled via the existing code in libxl_device_nic_add. I suppose what I could come up with is the kind of modification I was proposing in one of those emails last year (sounds like an awful long time :)). So at the point where the device-model is about to get started and the code already loops over the NIC definitions. That won't give the libvirt path default devid's but any other user that has not done yet. -Stefan -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] libxl: Fix devid init in libxlMakeNicList
Stefan Bader wrote:
This basically reverts commit ba64b97134a6129a48684f22f31be92c3b6eef96
libxl: Allow libxl to set NIC devid. However assigning devid's
before calling libxlMakeNic does not work as that is calling
libxl_device_nic_init which sets it back to -1.
Right now auto-assignment only works in the hotplug case. But even if
that would be fixed at some point (if that is possible at all), this
would add a weird dependency between Xen and libvirt versions.
Yeah, I had numerous inquires and bugs come my way even after fixing vfb and
vkb devid initialization in libxl with xen.git commit 5420f265. It took
some time for the fix to make its way to downstream users. Since there is
not yet a fix in Xen, it only makes sense to do the nic devid initialization
in libvirt to fix PXE booting HVM domains.
The change here should accept any auto-assignment that makes it into
libxl_device_nic_init. My understanding is that a caller always is
allowed to make the devid choice itself. And assuming libxlMakeNicList
is only used on domain creation, a sequential numbering should be ok.
Signed-off-by: Stefan Bader stefan.ba...@canonical.com
---
src/libxl/libxl_conf.c |7 +++
1 file changed, 7 insertions(+)
diff --git a/src/libxl/libxl_conf.c b/src/libxl/libxl_conf.c
index 04d01af..4cefadf 100644
--- a/src/libxl/libxl_conf.c
+++ b/src/libxl/libxl_conf.c
@@ -918,6 +918,13 @@ libxlMakeNicList(virDomainDefPtr def,
libxl_domain_config *d_config)
for (i = 0; i nnics; i++) {
if (libxlMakeNic(def, l_nics[i], x_nics[i]))
goto error;
+/*
+ * The devid (at least right now) will not get initialized by
+ * libxl in the setup case but is required for starting the
+ * device-model.
+ */
+if (x_nics[i].devid 0)
+x_nics[i].devid = i;
I think this is a better approach than the original code removed by ba64b971.
I've pushed this since it is clearly a bug fix and appropriate for 1.2.1.
Regards,
Jim
}
d_config-nics = x_nics;
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
