subject:"\[libvirt\] \[PATCH 17\/17\] qemu\: support type=hostdev network device hotplug attach\/detach"

Re: [libvirt] [PATCH 17/17] qemu: support type=hostdev network device hotplug attach/detach

2012-03-05 Thread Michal Privoznik

On 28.02.2012 21:14, Laine Stump wrote:
 qemuDomainAttachNetDevice
 
   - re-ordered some things at start of function because
 networkAllocateActualDevice should always be run and a slot
 in def-nets always allocated, but host_net_add isn't needed
 if the actual type is hostdev.
 
   - if actual type is hostdev, defer to
 qemuDomainAttachHostDevice (which will reach up to the NetDef
 for things like MAC address when necessary). After return
 from qemuDomainAttachHostDevice, slip directly to cleanup,
 since the rest of the function is specific to emulated net
 devices.
 
   - put assignment of new NetDef into expanded def-nets down
 below cleanup: (but only on success) since it is also needed
 for emulated and hostdev net devices.
 
 qemuDomainDetachHostDevice
 
   - after locating the exact device to detach, check if it's a
 network device and, if so, use toplevel
 qemuDomainDetachNetDevice instead so that the def-nets list
 is properly updated, and 'actual device' properly returned to
 network pool if appropriate. Otherwise, for normal hostdevs,
 call the lower level qemuDomainDetachThisDevice.
 
 qemuDomainDetachNetDevice
 
   - This is where it gets a bit tricky. After locating the device
 on the def-nets list, if the network device type == hostdev,
 call the *lower level* qemuDomainDetachThisDevice (which will
 reach back up to the parent net device for MAC address /
 virtualport when appropriate, then clear the device out of
 def-hostdevs) before skipping past all the emulated
 net-device-specific code to cleanup:, where the network
 device is removed from def-nets, and the network device
 object is freed.
 
 In short, any time a hostdev-type network device is detached, we must
 go through the toplevel virDomaineDetachNetDevice function first and
 last, to make sure 1) the def-nnets list is properly managed, and 2)
 any device allocated with networkAllocateActualDevice is properly
 freed. At the same time, in the middle we need to go through the
 lower-level virDomainDetach*This*HostDevice to be sure that 1) the
 def-hostdevs list is properly managed, 2) the PCI device is properly
 detached from the guest and reattached to the host (if appropriate),
 and 3) any higher level setup/teardown is called at the appropriate
 time, by reaching back up to the NetDef config (part (3) will be
 covered in a separate patch).
 
 ---
  src/qemu/qemu_hotplug.c |   61 +-
  1 files changed, 44 insertions(+), 17 deletions(-)
 
 diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c
 index 6119108..50563c5 100644
 --- a/src/qemu/qemu_hotplug.c
 +++ b/src/qemu/qemu_hotplug.c
 @@ -661,9 +661,9 @@ int qemuDomainAttachNetDevice(virConnectPtr conn,
  bool iface_connected = false;
  int actualType;
  
 -if (!qemuCapsGet(priv-qemuCaps, QEMU_CAPS_HOST_NET_ADD)) {
 -qemuReportError(VIR_ERR_CONFIG_UNSUPPORTED, %s,
 -_(installed qemu version does not support 
 host_net_add));
 +/* preallocate new slot for device */
 +if (VIR_REALLOC_N(vm-def-nets, vm-def-nnets+1)  0) {
 +virReportOOMError();
  return -1;
  }
  
 @@ -672,9 +672,27 @@ int qemuDomainAttachNetDevice(virConnectPtr conn,
   * to the one defined in the network definition.
   */
  if (networkAllocateActualDevice(net)  0)
 -goto cleanup;
 +return -1;

Okay, vm-def-nets won't leak, but will be one item bigger; Do we want
to realloc it back as we do in all detach functions, e.g.
virDomainNetRemove() ? The vm-def-nnets counter isn't changed yet so I
guess this is alright.

Anyway, looking good so ACK.

Michal

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

Re: [libvirt] [PATCH 17/17] qemu: support type=hostdev network device hotplug attach/detach

2012-03-05 Thread Laine Stump

On 03/05/2012 03:46 PM, Michal Privoznik wrote:
 On 28.02.2012 21:14, Laine Stump wrote:
 qemuDomainAttachNetDevice

   - re-ordered some things at start of function because
 networkAllocateActualDevice should always be run and a slot
 in def-nets always allocated, but host_net_add isn't needed
 if the actual type is hostdev.

   - if actual type is hostdev, defer to
 qemuDomainAttachHostDevice (which will reach up to the NetDef
 for things like MAC address when necessary). After return
 from qemuDomainAttachHostDevice, slip directly to cleanup,
 since the rest of the function is specific to emulated net
 devices.

   - put assignment of new NetDef into expanded def-nets down
 below cleanup: (but only on success) since it is also needed
 for emulated and hostdev net devices.

 qemuDomainDetachHostDevice

   - after locating the exact device to detach, check if it's a
 network device and, if so, use toplevel
 qemuDomainDetachNetDevice instead so that the def-nets list
 is properly updated, and 'actual device' properly returned to
 network pool if appropriate. Otherwise, for normal hostdevs,
 call the lower level qemuDomainDetachThisDevice.

 qemuDomainDetachNetDevice

   - This is where it gets a bit tricky. After locating the device
 on the def-nets list, if the network device type == hostdev,
 call the *lower level* qemuDomainDetachThisDevice (which will
 reach back up to the parent net device for MAC address /
 virtualport when appropriate, then clear the device out of
 def-hostdevs) before skipping past all the emulated
 net-device-specific code to cleanup:, where the network
 device is removed from def-nets, and the network device
 object is freed.

 In short, any time a hostdev-type network device is detached, we must
 go through the toplevel virDomaineDetachNetDevice function first and
 last, to make sure 1) the def-nnets list is properly managed, and 2)
 any device allocated with networkAllocateActualDevice is properly
 freed. At the same time, in the middle we need to go through the
 lower-level virDomainDetach*This*HostDevice to be sure that 1) the
 def-hostdevs list is properly managed, 2) the PCI device is properly
 detached from the guest and reattached to the host (if appropriate),
 and 3) any higher level setup/teardown is called at the appropriate
 time, by reaching back up to the NetDef config (part (3) will be
 covered in a separate patch).

 ---
  src/qemu/qemu_hotplug.c |   61 
 +-
  1 files changed, 44 insertions(+), 17 deletions(-)

 diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c
 index 6119108..50563c5 100644
 --- a/src/qemu/qemu_hotplug.c
 +++ b/src/qemu/qemu_hotplug.c
 @@ -661,9 +661,9 @@ int qemuDomainAttachNetDevice(virConnectPtr conn,
  bool iface_connected = false;
  int actualType;
  
 -if (!qemuCapsGet(priv-qemuCaps, QEMU_CAPS_HOST_NET_ADD)) {
 -qemuReportError(VIR_ERR_CONFIG_UNSUPPORTED, %s,
 -_(installed qemu version does not support 
 host_net_add));
 +/* preallocate new slot for device */
 +if (VIR_REALLOC_N(vm-def-nets, vm-def-nnets+1)  0) {
 +virReportOOMError();
  return -1;
  }
  
 @@ -672,9 +672,27 @@ int qemuDomainAttachNetDevice(virConnectPtr conn,
   * to the one defined in the network definition.
   */
  if (networkAllocateActualDevice(net)  0)
 -goto cleanup;
 +return -1;
 Okay, vm-def-nets won't leak, but will be one item bigger; Do we want
 to realloc it back as we do in all detach functions, e.g.
 virDomainNetRemove() ? The vm-def-nnets counter isn't changed yet so I
 guess this is alright.

I wondered about this too, but am just copying existing behavior which,
as you say, is relying on the fact that there's no leak, the item is
just one longer (and that one extra in length will be used up the next
time someone wants to grow the array).


 Anyway, looking good so ACK.


Thanks to you and to Eric for all the reviews!

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

[libvirt] [PATCH 17/17] qemu: support type=hostdev network device hotplug attach/detach

2012-02-28 Thread Laine Stump

qemuDomainAttachNetDevice

  - re-ordered some things at start of function because
networkAllocateActualDevice should always be run and a slot
in def-nets always allocated, but host_net_add isn't needed
if the actual type is hostdev.

  - if actual type is hostdev, defer to
qemuDomainAttachHostDevice (which will reach up to the NetDef
for things like MAC address when necessary). After return
from qemuDomainAttachHostDevice, slip directly to cleanup,
since the rest of the function is specific to emulated net
devices.

  - put assignment of new NetDef into expanded def-nets down
below cleanup: (but only on success) since it is also needed
for emulated and hostdev net devices.

qemuDomainDetachHostDevice

  - after locating the exact device to detach, check if it's a
network device and, if so, use toplevel
qemuDomainDetachNetDevice instead so that the def-nets list
is properly updated, and 'actual device' properly returned to
network pool if appropriate. Otherwise, for normal hostdevs,
call the lower level qemuDomainDetachThisDevice.

qemuDomainDetachNetDevice

  - This is where it gets a bit tricky. After locating the device
on the def-nets list, if the network device type == hostdev,
call the *lower level* qemuDomainDetachThisDevice (which will
reach back up to the parent net device for MAC address /
virtualport when appropriate, then clear the device out of
def-hostdevs) before skipping past all the emulated
net-device-specific code to cleanup:, where the network
device is removed from def-nets, and the network device
object is freed.

In short, any time a hostdev-type network device is detached, we must
go through the toplevel virDomaineDetachNetDevice function first and
last, to make sure 1) the def-nnets list is properly managed, and 2)
any device allocated with networkAllocateActualDevice is properly
freed. At the same time, in the middle we need to go through the
lower-level virDomainDetach*This*HostDevice to be sure that 1) the
def-hostdevs list is properly managed, 2) the PCI device is properly
detached from the guest and reattached to the host (if appropriate),
and 3) any higher level setup/teardown is called at the appropriate
time, by reaching back up to the NetDef config (part (3) will be
covered in a separate patch).

---
 src/qemu/qemu_hotplug.c |   61 +-
 1 files changed, 44 insertions(+), 17 deletions(-)

diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c
index 6119108..50563c5 100644
--- a/src/qemu/qemu_hotplug.c
+++ b/src/qemu/qemu_hotplug.c
@@ -661,9 +661,9 @@ int qemuDomainAttachNetDevice(virConnectPtr conn,
 bool iface_connected = false;
 int actualType;
 
-if (!qemuCapsGet(priv-qemuCaps, QEMU_CAPS_HOST_NET_ADD)) {
-qemuReportError(VIR_ERR_CONFIG_UNSUPPORTED, %s,
-_(installed qemu version does not support 
host_net_add));
+/* preallocate new slot for device */
+if (VIR_REALLOC_N(vm-def-nets, vm-def-nnets+1)  0) {
+virReportOOMError();
 return -1;
 }
 
@@ -672,9 +672,27 @@ int qemuDomainAttachNetDevice(virConnectPtr conn,
  * to the one defined in the network definition.
  */
 if (networkAllocateActualDevice(net)  0)
-goto cleanup;
+return -1;
 
 actualType = virDomainNetGetActualType(net);
+
+if (actualType == VIR_DOMAIN_NET_TYPE_HOSTDEV) {
+/* This is really a smart hostdev, so it should be attached
+ * as a hostdev (the hostdev code will reach over into the
+ * netdev-specific code as appropriate), then also added to
+ * the nets list (see cleanup:) if successful.
+ */
+ret = qemuDomainAttachHostDevice(driver, vm,
+ virDomainNetGetActualHostdev(net));
+goto cleanup;
+}
+
+if (!qemuCapsGet(priv-qemuCaps, QEMU_CAPS_HOST_NET_ADD)) {
+qemuReportError(VIR_ERR_CONFIG_UNSUPPORTED, %s,
+_(installed qemu version does not support 
host_net_add));
+goto cleanup;
+}
+
 if (actualType == VIR_DOMAIN_NET_TYPE_BRIDGE ||
 actualType == VIR_DOMAIN_NET_TYPE_NETWORK) {
 if ((tapfd = qemuNetworkIfaceConnect(vm-def, conn, driver, net,
@@ -693,9 +711,6 @@ int qemuDomainAttachNetDevice(virConnectPtr conn,
 goto cleanup;
 }
 
-if (VIR_REALLOC_N(vm-def-nets, vm-def-nnets+1)  0)
-goto no_memory;
-
 if (qemuCapsGet(priv-qemuCaps, QEMU_CAPS_NET_NAME) ||
 qemuCapsGet(priv-qemuCaps, QEMU_CAPS_DEVICE)) {
 if (qemuAssignDeviceNetAlias(vm-def, net, -1)  0)
@@ -826,10 +841,10 @@ int qemuDomainAttachNetDevice(virConnectPtr conn,
 
 ret = 0;
 
-vm-def-nets[vm-def-nnets++] = net;
-
 cleanup:
-if (ret  0) {
+if (!ret) {
+vm-def-nets[vm-def-nnets++] = net;
+} else {
 if (qemuCapsGet(priv-qemuCaps, QEMU_CAPS_DEVICE)

Re: [libvirt] [PATCH 17/17] qemu: support type=hostdev network device hotplug attach/detach

Re: [libvirt] [PATCH 17/17] qemu: support type=hostdev network device hotplug attach/detach

[libvirt] [PATCH 17/17] qemu: support type=hostdev network device hotplug attach/detach

3 matches

Site Navigation

Mail list logo

Footer information