Re: [libvirt] [PATCH v1] libxl: reverse defaults on HVM net device attach

2016-12-15 Thread Joao Martins


On 12/14/2016 08:53 PM, Jim Fehlig wrote:
> On 12/14/2016 03:53 AM, Joao Martins wrote:
>> libvirt libxl picks its own default with respect to the default NIC
>> to use. libxlMakeNic is the one responsible for this and on boot it
>> picks LIBXL_NIC_TYPE_VIF_IOEMU for HVM domains such that it accomodates
>> both PV and emulated one. The good behaving guest at boot will then
>> select the pv and unplug the emulated device.
>>
>> Now, on HVM when attaching an interface it will pick the same default
>> that is LIBXL_NIC_TYPE_VIF_IOEMU which as a result will fail the attach
>> (see xen commit 32e9d0f ("libxl: nic type defaults to vif in hotplug for
>> hvm guest"). Xen doesn't yet support the hotplug of emulated devices,
>> but we don't want to rule out that case either, which might get support
>> in the future. Hence we simply reverse the defaults when we are
>> attaching the interface which allows libvirt to prefer the PV nic first
>> without adding "model='netfront'" following the same pattern as above
>> commit. Also to avoid ruling out the emulated one we set to
>> LIBXL_NIC_TYPE_IOEMU when setting a model type that is not 'netfront'.
>>
>> Signed-off-by: Joao Martins 
>> Signed-off-by: Jim Fehlig 
>> ---
>> Since RFC:
>> - Joao: Add mention to HVM domains in the first paragraph of commit
>> message.
>> - Jim: Add comment about libxl_device_nic type inner workings
>> - Jim: Prevent attach on PV when model != netfront
>> - Joao: Remove now unused ioemu_nic variable
>> - Joao: Append one small paragraph about why we mimic xl/libxl
>> ---
>>  src/libxl/libxl_conf.c   | 39 +++
>>  src/libxl/libxl_conf.h   |  3 ++-
>>  src/libxl/libxl_driver.c |  2 +-
>>  3 files changed, 34 insertions(+), 10 deletions(-)
> 
> ACK and pushed.
Thank you!

Joao

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list


Re: [libvirt] [PATCH v1] libxl: reverse defaults on HVM net device attach

2016-12-14 Thread Jim Fehlig

On 12/14/2016 03:53 AM, Joao Martins wrote:

libvirt libxl picks its own default with respect to the default NIC
to use. libxlMakeNic is the one responsible for this and on boot it
picks LIBXL_NIC_TYPE_VIF_IOEMU for HVM domains such that it accomodates
both PV and emulated one. The good behaving guest at boot will then
select the pv and unplug the emulated device.

Now, on HVM when attaching an interface it will pick the same default
that is LIBXL_NIC_TYPE_VIF_IOEMU which as a result will fail the attach
(see xen commit 32e9d0f ("libxl: nic type defaults to vif in hotplug for
hvm guest"). Xen doesn't yet support the hotplug of emulated devices,
but we don't want to rule out that case either, which might get support
in the future. Hence we simply reverse the defaults when we are
attaching the interface which allows libvirt to prefer the PV nic first
without adding "model='netfront'" following the same pattern as above
commit. Also to avoid ruling out the emulated one we set to
LIBXL_NIC_TYPE_IOEMU when setting a model type that is not 'netfront'.

Signed-off-by: Joao Martins 
Signed-off-by: Jim Fehlig 
---
Since RFC:
- Joao: Add mention to HVM domains in the first paragraph of commit
message.
- Jim: Add comment about libxl_device_nic type inner workings
- Jim: Prevent attach on PV when model != netfront
- Joao: Remove now unused ioemu_nic variable
- Joao: Append one small paragraph about why we mimic xl/libxl
---
 src/libxl/libxl_conf.c   | 39 +++
 src/libxl/libxl_conf.h   |  3 ++-
 src/libxl/libxl_driver.c |  2 +-
 3 files changed, 34 insertions(+), 10 deletions(-)


ACK and pushed.

Regards,
Jim

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list


[libvirt] [PATCH v1] libxl: reverse defaults on HVM net device attach

2016-12-14 Thread Joao Martins
libvirt libxl picks its own default with respect to the default NIC
to use. libxlMakeNic is the one responsible for this and on boot it
picks LIBXL_NIC_TYPE_VIF_IOEMU for HVM domains such that it accomodates
both PV and emulated one. The good behaving guest at boot will then
select the pv and unplug the emulated device.

Now, on HVM when attaching an interface it will pick the same default
that is LIBXL_NIC_TYPE_VIF_IOEMU which as a result will fail the attach
(see xen commit 32e9d0f ("libxl: nic type defaults to vif in hotplug for
hvm guest"). Xen doesn't yet support the hotplug of emulated devices,
but we don't want to rule out that case either, which might get support
in the future. Hence we simply reverse the defaults when we are
attaching the interface which allows libvirt to prefer the PV nic first
without adding "model='netfront'" following the same pattern as above
commit. Also to avoid ruling out the emulated one we set to
LIBXL_NIC_TYPE_IOEMU when setting a model type that is not 'netfront'.

Signed-off-by: Joao Martins 
Signed-off-by: Jim Fehlig 
---
Since RFC:
- Joao: Add mention to HVM domains in the first paragraph of commit
message.
- Jim: Add comment about libxl_device_nic type inner workings
- Jim: Prevent attach on PV when model != netfront
- Joao: Remove now unused ioemu_nic variable
- Joao: Append one small paragraph about why we mimic xl/libxl
---
 src/libxl/libxl_conf.c   | 39 +++
 src/libxl/libxl_conf.h   |  3 ++-
 src/libxl/libxl_driver.c |  2 +-
 3 files changed, 34 insertions(+), 10 deletions(-)

diff --git a/src/libxl/libxl_conf.c b/src/libxl/libxl_conf.c
index dcf8e7e..9515e2a 100644
--- a/src/libxl/libxl_conf.c
+++ b/src/libxl/libxl_conf.c
@@ -881,9 +881,9 @@ libxlMakeDiskList(virDomainDefPtr def, libxl_domain_config 
*d_config)
 int
 libxlMakeNic(virDomainDefPtr def,
  virDomainNetDefPtr l_nic,
- libxl_device_nic *x_nic)
+ libxl_device_nic *x_nic,
+ bool attach)
 {
-bool ioemu_nic = def->os.type == VIR_DOMAIN_OSTYPE_HVM;
 virDomainNetType actual_type = virDomainNetGetActualType(l_nic);
 virNetworkPtr network = NULL;
 virConnectPtr conn = NULL;
@@ -907,16 +907,39 @@ libxlMakeNic(virDomainDefPtr def,
 
 virMacAddrGetRaw(&l_nic->mac, x_nic->mac);
 
-if (ioemu_nic)
-x_nic->nictype = LIBXL_NIC_TYPE_VIF_IOEMU;
-else
-x_nic->nictype = LIBXL_NIC_TYPE_VIF;
-
+/*
+ * The nictype field of libxl_device_nic structure tells Xen which type of
+ * NIC device to create for the domain. LIBXL_NIC_TYPE_VIF specifies a
+ * PV NIC. LIBXL_NIC_TYPE_VIF_IOEMU specifies a PV and emulated NIC,
+ * allowing the domain to choose which NIC to use and unplug the unused
+ * one. LIBXL_NIC_TYPE_VIF_IOEMU is only valid for HVM domains. Further,
+ * if hotplugging the NIC, emulated NICs are currently not supported.
+ * Alternatively one could set LIBXL_NIC_TYPE_UNKNOWN and let libxl decide,
+ * but its behaviour might not be consistent across all libvirt supported
+ * versions. The other nictype values are well established already, hence
+ * we manually select our own default and mimic xl/libxl behaviour starting
+ * xen commit 32e9d0f ("libxl: nic type defaults to vif in hotplug for
+ * hvm guest").
+ */
 if (l_nic->model) {
+if (def->os.type == VIR_DOMAIN_OSTYPE_XEN &&
+STRNEQ(l_nic->model, "netfront")) {
+virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
+   _("only model 'netfront' is supported for "
+ "Xen PV domains"));
+return -1;
+}
 if (VIR_STRDUP(x_nic->model, l_nic->model) < 0)
 goto cleanup;
 if (STREQ(l_nic->model, "netfront"))
 x_nic->nictype = LIBXL_NIC_TYPE_VIF;
+else
+x_nic->nictype = LIBXL_NIC_TYPE_VIF_IOEMU;
+} else {
+if (def->os.type == VIR_DOMAIN_OSTYPE_HVM && !attach)
+x_nic->nictype = LIBXL_NIC_TYPE_VIF_IOEMU;
+else
+x_nic->nictype = LIBXL_NIC_TYPE_VIF;
 }
 
 if (VIR_STRDUP(x_nic->ifname, l_nic->ifname) < 0)
@@ -1047,7 +1070,7 @@ libxlMakeNicList(virDomainDefPtr def,  
libxl_domain_config *d_config)
 if (virDomainNetGetActualType(l_nics[i]) == 
VIR_DOMAIN_NET_TYPE_HOSTDEV)
 continue;
 
-if (libxlMakeNic(def, l_nics[i], &x_nics[nvnics]))
+if (libxlMakeNic(def, l_nics[i], &x_nics[nvnics], false))
 goto error;
 /*
  * The devid (at least right now) will not get initialized by
diff --git a/src/libxl/libxl_conf.h b/src/libxl/libxl_conf.h
index 0ea76b4..851f3af 100644
--- a/src/libxl/libxl_conf.h
+++ b/src/libxl/libxl_conf.h
@@ -178,7 +178,8 @@ libxlMakeDisk(virDomainDiskDefPtr l_dev, libxl_device_disk 
*x_dev);
 int
 libxlMakeNic(virDomainDefPtr def,
  virDomainNetDefPtr l_nic,
- libxl_device_nic *x_nic