[libvirt] [PATCH v2 11/14] libxl: support network device attach/detach
Jim Fehlig
jfehlig at suse.com
Wed Jul 3 21:43:45 UTC 2013
On 06/12/2013 07:54 PM, Marek Marczykowski-Górecki wrote:
> Both live and config.
>
> Signed-off-by: Marek Marczykowski-Górecki <marmarek at invisiblethingslab.com>
> ---
> src/libxl/libxl_driver.c | 164 ++++++++++++++++++++++++++++++++++++++++++++++-
> 1 file changed, 161 insertions(+), 3 deletions(-)
>
> diff --git a/src/libxl/libxl_driver.c b/src/libxl/libxl_driver.c
> index 7b50853..ae0d4f7 100644
> --- a/src/libxl/libxl_driver.c
> +++ b/src/libxl/libxl_driver.c
> @@ -3417,6 +3417,111 @@ cleanup:
> }
>
> static int
> +libxlDomainAttachDeviceNetLive(libxlDriverPrivatePtr driver,
> + libxlDomainObjPrivatePtr priv,
> + virDomainObjPtr vm,
> + virDomainDeviceDefPtr dev)
> +{
> + virDomainNetDefPtr l_net = dev->data.net;
> + libxl_device_nic x_nic;
> + char mac[VIR_MAC_STRING_BUFLEN];
> + int ret = -1;
> +
> + switch (dev->data.net->type) {
l_net->type ?
> + case VIR_DOMAIN_NET_TYPE_ETHERNET:
> + case VIR_DOMAIN_NET_TYPE_BRIDGE:
> + /* -2 means "multiple matches" so then fail also */
> + if (virDomainNetFindIdx(vm->def, dev->data.net) != -1) {
And just pass l_net here. Also, you should check for -2. I think this is
better coded as
idx = virDomainNetFindIdx(vm->def, l_net);
if (idx == -2) {
virReportError(VIR_ERR_OPERATION_FAILED,
_("multiple devices matching mac address %s found"),
virMacAddrFormat(&net->mac, mac));
return -1;
} else if (idx < 0) {
virReportError(VIR_ERR_OPERATION_FAILED, "%s",
_("no matching network device was found"));
return -1;
}
> + virReportError(VIR_ERR_OPERATION_FAILED,
> + _("device matching mac address %s already exists"),
We should use the same error message as the qemu driver, which is included in my
snippet above.
> + virMacAddrFormat(&dev->data.net->mac, mac));
> + goto cleanup;
> + }
> +
> + if (libxlMakeNic(driver, l_net, &x_nic) < 0)
> + goto cleanup;
> +
> + if ((ret = libxl_device_nic_add(priv->ctx, vm->def->id,
> + &x_nic, NULL)) < 0) {
That indentation looks a little odd. Perhaps best to split into 2 lines
ret = libxl_deivce_nic_add(...);
if (ret < 0)
> + virReportError(VIR_ERR_INTERNAL_ERROR,
> + _("libxenlight failed to attach net '%s'"),
> + virMacAddrFormat(&dev->data.net->mac, mac));
> + goto cleanup;
> + }
> +
> + virDomainNetInsert(vm->def, l_net);
> +
> + break;
> + default:
> + virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
> + _("net device type '%s' cannot be hotplugged"),
s/net/network/
> + virDomainNetTypeToString(l_net->type));
> + break;
> + }
> +
> +cleanup:
> + return ret;
Nothing to cleanup. Might as well just return errors where they occur or
success when done.
> +}
> +
> +static int
> +libxlDomainDetachDeviceNetLive(libxlDriverPrivatePtr driver,
> + libxlDomainObjPrivatePtr priv,
> + virDomainObjPtr vm,
> + virDomainDeviceDefPtr dev)
> +{
> + virDomainNetDefPtr l_net = NULL;
> + libxl_device_nic x_nic;
> + char mac[VIR_MAC_STRING_BUFLEN];
> + int i;
> + int ret = -1;
> +
> + switch (dev->data.net->type) {
> + case VIR_DOMAIN_NET_TYPE_ETHERNET:
> + case VIR_DOMAIN_NET_TYPE_BRIDGE:
> + if ((i = virDomainNetFindIdx(vm->def, dev->data.net)) < 0) {
> + if (i == -2) {
> +
> + virReportError(VIR_ERR_OPERATION_FAILED,
> + _("multiple devices matching mac address %s found"),
> + virMacAddrFormat(&dev->data.net->mac, mac));
> + }
> + else {
> + virReportError(VIR_ERR_OPERATION_FAILED,
> + _("network device %s not found"),
> + virMacAddrFormat(&dev->data.net->mac, mac));
> + }
> + goto cleanup;
> + }
For consistency, the error checking logic here should be changed to match the
Attach function.
> +
> + l_net = vm->def->nets[i];
> +
> + if (libxlMakeNic(driver, l_net, &x_nic) < 0)
> + goto cleanup;
> +
> + if ((ret = libxl_device_nic_remove(priv->ctx, vm->def->id,
> + &x_nic, NULL)) < 0) {
Split into 2 lines?
> + virReportError(VIR_ERR_INTERNAL_ERROR,
> + _("libxenlight failed to detach nic '%d'"),
> + i);
> + goto cleanup;
> + }
> +
> + virDomainNetRemove(vm->def, i);
> + virDomainNetDefFree(l_net);
> +
> + break;
> + default:
> + virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
> + _("net device type '%s' cannot hot unplugged"),
s/net/network/
> + virDomainNetTypeToString(dev->data.net->type));
> + break;
> + }
> +
> +cleanup:
> + return ret;
Nothing to cleanup.
> +}
> +
> +static int
> libxlDomainAttachDeviceLive(libxlDriverPrivatePtr driver,
> libxlDomainObjPrivatePtr priv, virDomainObjPtr vm,
> virDomainDeviceDefPtr dev)
> @@ -3430,6 +3535,12 @@ libxlDomainAttachDeviceLive(libxlDriverPrivatePtr driver,
> dev->data.disk = NULL;
> break;
>
> + case VIR_DOMAIN_DEVICE_NET:
> + ret = libxlDomainAttachDeviceNetLive(driver, priv, vm, dev);
> + if (!ret)
> + dev->data.net = NULL;
> + break;
> +
> default:
> virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
> _("device type '%s' cannot be attached"),
> @@ -3444,6 +3555,8 @@ static int
> libxlDomainAttachDeviceConfig(virDomainDefPtr vmdef, virDomainDeviceDefPtr dev)
> {
> virDomainDiskDefPtr disk;
> + virDomainNetDefPtr net;
> + char mac[VIR_MAC_STRING_BUFLEN];
>
> switch (dev->type) {
> case VIR_DOMAIN_DEVICE_DISK:
> @@ -3461,6 +3574,22 @@ libxlDomainAttachDeviceConfig(virDomainDefPtr vmdef, virDomainDeviceDefPtr dev)
> dev->data.disk = NULL;
> break;
>
> + case VIR_DOMAIN_DEVICE_NET:
> + net = dev->data.net;
> + if (virDomainNetFindIdx(vmdef, net) >= 0) {
> + virReportError(VIR_ERR_INVALID_ARG,
> + _("net device with mac %s already exists."),
Reuse existing error message.
> + virMacAddrFormat(&net->mac, mac));
> + return -1;
> + }
> + if (virDomainNetInsert(vmdef, net)) {
> + virReportOOMError();
> + return -1;
> + }
> + /* vmdef has the pointer. Generic codes for vmdef will do all jobs */
> + dev->data.net = NULL;
> + break;
> +
> default:
> virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
> _("persistent attach of device is not supported"));
> @@ -3482,6 +3611,10 @@ libxlDomainDetachDeviceLive(libxlDriverPrivatePtr driver,
> ret = libxlDomainDetachDeviceDiskLive(driver, priv, vm, dev);
> break;
>
> + case VIR_DOMAIN_DEVICE_NET:
> + ret = libxlDomainDetachDeviceNetLive(driver, priv, vm, dev);
> + break;
> +
> default:
> virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
> _("device type '%s' cannot be detached"),
> @@ -3495,20 +3628,45 @@ libxlDomainDetachDeviceLive(libxlDriverPrivatePtr driver,
> static int
> libxlDomainDetachDeviceConfig(virDomainDefPtr vmdef, virDomainDeviceDefPtr dev)
> {
> - virDomainDiskDefPtr disk, detach;
> + virDomainDiskDefPtr disk, disk_detach;
> + virDomainNetDefPtr net, net_detach;
> + char mac[VIR_MAC_STRING_BUFLEN];
> + int net_idx;
> int ret = -1;
>
> switch (dev->type) {
> case VIR_DOMAIN_DEVICE_DISK:
> disk = dev->data.disk;
> - if (!(detach = virDomainDiskRemoveByName(vmdef, disk->dst))) {
> + if (!(disk_detach = virDomainDiskRemoveByName(vmdef, disk->dst))) {
> virReportError(VIR_ERR_INVALID_ARG,
> _("no target device %s"), disk->dst);
> break;
> }
> - virDomainDiskDefFree(detach);
> + virDomainDiskDefFree(disk_detach);
> + ret = 0;
> + break;
> +
> + case VIR_DOMAIN_DEVICE_NET:
> + net = dev->data.net;
> + if ((net_idx = virDomainNetFindIdx(vmdef, net)) < 0) {
> + if (net_idx == -2) {
> +
> + virReportError(VIR_ERR_OPERATION_FAILED,
> + _("multiple devices matching mac address %s found"),
> + virMacAddrFormat(&dev->data.net->mac, mac));
> + }
> + else {
> + virReportError(VIR_ERR_OPERATION_FAILED,
> + _("network device %s not found"),
> + virMacAddrFormat(&dev->data.net->mac, mac));
> + }
> + return -1;
> + }
Again for consistency, same error checking logic here.
Regards,
Jim
> + net_detach = virDomainNetRemove(vmdef, net_idx);
> + virDomainNetDefFree(net_detach);
> ret = 0;
> break;
> +
> default:
> virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
> _("persistent detach of device is not supported"));
More information about the libvir-list
mailing list