On 04/18/2016 02:01 PM, Kothapally Madhu Pavan wrote:
> When we try to detach a network device without specifying
> the mac address, random mac address is generated. As the
> generated mac address will not be available in the running
> vm, detaching device will fail erroring out "error:
> operation failed: no device matching mac address
> xx:xx:xx:xx:xx:xx found".
Been on list for a bit without attention - so I'll bite...
Could you perhaps provide some more specifics? Example that's failing
would really help and of course your thoughts about the next paragraph.
I would think there could be a concern over *matching* which of many
networks was being attempted to be detached. The 'detach-interface'
virsh page specifically mentions it is recommended to use the mac to
distinguish between the interfaces if more than one are present on the
domain.
>
> Signed-off-by: Kothapally Madhu Pavan <kmp@xxxxxxxxxxxxxxxxxx>
> ---
> src/conf/domain_conf.c | 4 ++++
> src/conf/domain_conf.h | 3 +++
> src/libxl/libxl_driver.c | 12 ++++++------
> src/lxc/lxc_driver.c | 7 ++++---
> src/qemu/qemu_driver.c | 2 ++
> src/uml/uml_driver.c | 6 ++++--
> src/vbox/vbox_common.c | 6 ++++--
> src/vz/vz_driver.c | 4 +++-
> src/xen/xend_internal.c | 6 ++++--
> src/xen/xm_internal.c | 8 ++++----
> 10 files changed, 38 insertions(+), 20 deletions(-)
>
> diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
> index 28248c8..512d877 100644
> --- a/src/conf/domain_conf.c
> +++ b/src/conf/domain_conf.c
> @@ -8784,6 +8784,10 @@ virDomainNetDefParseXML(virDomainXMLOptionPtr xmlopt,
> (const char *)macaddr);
> goto error;
> }
> + } else if (flags & VIR_DOMAIN_DEF_PARSE_SKIP_GENERATE_MACADDR) {
> + virReportError(VIR_ERR_XML_ERROR, "%s",
> + _("mac address not specified in the device xml"));
> + goto error;
So from any Detach path which you added the flag, if the incoming XML
doesn't have a macaddr, then you get this failure?
I guess from the name I expected :
} else if !(flags & VIR_DOMAIN_DEF_PARSE_SKIP_GENERATE_MACADDR) {
virDomainNetGenerateMAC
That is - if I don't have the flag set, then generate the MAC;
otherwise, continue on parsing the XML... Of course we'll still fail later.
So, I think the better option is, set a flag when we *do* generate the
MAC, then in qemuDomainDetachNetDevice check that generated a mac flag
in order to send a bool to virDomainNetFindIdx to indicate matching the
mac isn't required (expected) and that allows the attempt to match
"other" fields (if possible).
Hopefully that's clear - it's been a long day.
John
> } else {
> virDomainNetGenerateMAC(xmlopt, &def->mac);
> }
> diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h
> index 1986f53..74692f1 100644
> --- a/src/conf/domain_conf.h
> +++ b/src/conf/domain_conf.h
> @@ -2679,6 +2679,9 @@ typedef enum {
> VIR_DOMAIN_DEF_PARSE_SKIP_OSTYPE_CHECKS = 1 << 8,
> /* allow updates in post parse callback that would break ABI otherwise */
> VIR_DOMAIN_DEF_PARSE_ABI_UPDATE = 1 << 9,
> + /* don't generate random mac address when a network device without mac address
> + * is detached */
> + VIR_DOMAIN_DEF_PARSE_SKIP_GENERATE_MACADDR = 1 << 10,
> } virDomainDefParseFlags;
>
> typedef enum {
> diff --git a/src/libxl/libxl_driver.c b/src/libxl/libxl_driver.c
> index bf97c9c..507edcf 100644
> --- a/src/libxl/libxl_driver.c
> +++ b/src/libxl/libxl_driver.c
> @@ -3726,6 +3726,8 @@ libxlDomainDetachDeviceFlags(virDomainPtr dom, const char *xml,
> virDomainDefPtr vmdef = NULL;
> virDomainDeviceDefPtr dev = NULL;
> int ret = -1;
> + unsigned int parse_flags = VIR_DOMAIN_DEF_PARSE_INACTIVE |
> + VIR_DOMAIN_DEF_PARSE_SKIP_GENERATE_MACADDR;
>
> virCheckFlags(VIR_DOMAIN_DEVICE_MODIFY_LIVE |
> VIR_DOMAIN_DEVICE_MODIFY_CONFIG, -1);
> @@ -3743,9 +3745,8 @@ libxlDomainDetachDeviceFlags(virDomainPtr dom, const char *xml,
> goto endjob;
>
> if (flags & VIR_DOMAIN_DEVICE_MODIFY_CONFIG) {
> - if (!(dev = virDomainDeviceDefParse(xml, vm->def,
> - cfg->caps, driver->xmlopt,
> - VIR_DOMAIN_DEF_PARSE_INACTIVE)))
> + if (!(dev = virDomainDeviceDefParse(xml, vm->def, cfg->caps,
> + driver->xmlopt, parse_flags)))
> goto endjob;
>
> /* Make a copy for updated domain. */
> @@ -3760,9 +3761,8 @@ libxlDomainDetachDeviceFlags(virDomainPtr dom, const char *xml,
> if (flags & VIR_DOMAIN_DEVICE_MODIFY_LIVE) {
> /* If dev exists it was created to modify the domain config. Free it. */
> virDomainDeviceDefFree(dev);
> - if (!(dev = virDomainDeviceDefParse(xml, vm->def,
> - cfg->caps, driver->xmlopt,
> - VIR_DOMAIN_DEF_PARSE_INACTIVE)))
> + if (!(dev = virDomainDeviceDefParse(xml, vm->def, cfg->caps,
> + driver->xmlopt, parse_flags)))
> goto endjob;
>
> if (libxlDomainDetachDeviceLive(driver, vm, dev) < 0)
> diff --git a/src/lxc/lxc_driver.c b/src/lxc/lxc_driver.c
> index ef48812..23f0d80 100644
> --- a/src/lxc/lxc_driver.c
> +++ b/src/lxc/lxc_driver.c
> @@ -5196,6 +5196,8 @@ static int lxcDomainDetachDeviceFlags(virDomainPtr dom,
> virDomainDefPtr vmdef = NULL;
> virDomainDeviceDefPtr dev = NULL, dev_copy = NULL;
> int ret = -1;
> + unsigned int parse_flags = VIR_DOMAIN_DEF_PARSE_INACTIVE |
> + VIR_DOMAIN_DEF_PARSE_SKIP_GENERATE_MACADDR;
> virLXCDriverConfigPtr cfg = virLXCDriverGetConfig(driver);
>
> virCheckFlags(VIR_DOMAIN_AFFECT_LIVE |
> @@ -5213,9 +5215,8 @@ static int lxcDomainDetachDeviceFlags(virDomainPtr dom,
> if (!(caps = virLXCDriverGetCapabilities(driver, false)))
> goto cleanup;
>
> - dev = dev_copy = virDomainDeviceDefParse(xml, vm->def,
> - caps, driver->xmlopt,
> - VIR_DOMAIN_DEF_PARSE_INACTIVE);
> + dev = dev_copy = virDomainDeviceDefParse(xml, vm->def, caps,
> + driver->xmlopt, parse_flags);
> if (dev == NULL)
> goto cleanup;
>
> diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
> index 862c44c..df85fd5 100644
> --- a/src/qemu/qemu_driver.c
> +++ b/src/qemu/qemu_driver.c
> @@ -8486,6 +8486,8 @@ static int qemuDomainDetachDeviceFlags(virDomainPtr dom, const char *xml,
> !(flags & VIR_DOMAIN_AFFECT_LIVE))
> parse_flags |= VIR_DOMAIN_DEF_PARSE_INACTIVE;
>
> + parse_flags |= VIR_DOMAIN_DEF_PARSE_SKIP_GENERATE_MACADDR;
> +
> dev = dev_copy = virDomainDeviceDefParse(xml, vm->def,
> caps, driver->xmlopt,
> parse_flags);
> diff --git a/src/uml/uml_driver.c b/src/uml/uml_driver.c
> index 84e1df8..c232435 100644
> --- a/src/uml/uml_driver.c
> +++ b/src/uml/uml_driver.c
> @@ -2346,6 +2346,8 @@ static int umlDomainDetachDevice(virDomainPtr dom, const char *xml)
> virDomainObjPtr vm;
> virDomainDeviceDefPtr dev = NULL;
> int ret = -1;
> + unsigned int parse_flags = VIR_DOMAIN_DEF_PARSE_INACTIVE |
> + VIR_DOMAIN_DEF_PARSE_SKIP_GENERATE_MACADDR;
>
> umlDriverLock(driver);
> vm = virDomainObjListFindByUUID(driver->domains, dom->uuid);
> @@ -2366,8 +2368,8 @@ static int umlDomainDetachDevice(virDomainPtr dom, const char *xml)
> goto cleanup;
> }
>
> - dev = virDomainDeviceDefParse(xml, vm->def, driver->caps, driver->xmlopt,
> - VIR_DOMAIN_DEF_PARSE_INACTIVE);
> + dev = virDomainDeviceDefParse(xml, vm->def, driver->caps,
> + driver->xmlopt, parse_flags);
> if (dev == NULL)
> goto cleanup;
>
> diff --git a/src/vbox/vbox_common.c b/src/vbox/vbox_common.c
> index 0cead10..4c539ef 100644
> --- a/src/vbox/vbox_common.c
> +++ b/src/vbox/vbox_common.c
> @@ -4228,6 +4228,8 @@ static int vboxDomainDetachDevice(virDomainPtr dom, const char *xml)
> virDomainDeviceDefPtr dev = NULL;
> nsresult rc;
> int ret = -1;
> + unsigned int parse_flags = VIR_DOMAIN_DEF_PARSE_INACTIVE |
> + VIR_DOMAIN_DEF_PARSE_SKIP_GENERATE_MACADDR;
>
> if (!data->vboxObj)
> return ret;
> @@ -4238,8 +4240,8 @@ static int vboxDomainDetachDevice(virDomainPtr dom, const char *xml)
>
> def->os.type = VIR_DOMAIN_OSTYPE_HVM;
>
> - dev = virDomainDeviceDefParse(xml, def, data->caps, data->xmlopt,
> - VIR_DOMAIN_DEF_PARSE_INACTIVE);
> + dev = virDomainDeviceDefParse(xml, def, data->caps,
> + data->xmlopt, parse_flags);
> if (dev == NULL)
> goto cleanup;
>
> diff --git a/src/vz/vz_driver.c b/src/vz/vz_driver.c
> index ffa6f45..f5f5395 100644
> --- a/src/vz/vz_driver.c
> +++ b/src/vz/vz_driver.c
> @@ -1172,6 +1172,8 @@ static int vzDomainDetachDeviceFlags(virDomainPtr dom, const char *xml,
> vzConnPtr privconn = dom->conn->privateData;
> virDomainDeviceDefPtr dev = NULL;
> virDomainObjPtr privdom = NULL;
> + unsigned int parse_flags = VIR_DOMAIN_XML_INACTIVE |
> + VIR_DOMAIN_DEF_PARSE_SKIP_GENERATE_MACADDR;
>
> virCheckFlags(VIR_DOMAIN_AFFECT_LIVE |
> VIR_DOMAIN_AFFECT_CONFIG, -1);
> @@ -1184,7 +1186,7 @@ static int vzDomainDetachDeviceFlags(virDomainPtr dom, const char *xml,
> goto cleanup;
>
> dev = virDomainDeviceDefParse(xml, privdom->def, privconn->driver->caps,
> - privconn->driver->xmlopt, VIR_DOMAIN_XML_INACTIVE);
> + privconn->driver->xmlopt, parse_flags);
> if (dev == NULL)
> goto cleanup;
>
> diff --git a/src/xen/xend_internal.c b/src/xen/xend_internal.c
> index cf7cdd0..216c4f4 100644
> --- a/src/xen/xend_internal.c
> +++ b/src/xen/xend_internal.c
> @@ -2327,6 +2327,8 @@ xenDaemonDetachDeviceFlags(virConnectPtr conn,
> int ret = -1;
> char *xendev = NULL;
> virBuffer buf = VIR_BUFFER_INITIALIZER;
> + unsigned int parse_flags = VIR_DOMAIN_DEF_PARSE_INACTIVE |
> + VIR_DOMAIN_DEF_PARSE_SKIP_GENERATE_MACADDR;
>
> virCheckFlags(VIR_DOMAIN_AFFECT_LIVE | VIR_DOMAIN_AFFECT_CONFIG, -1);
>
> @@ -2354,8 +2356,8 @@ xenDaemonDetachDeviceFlags(virConnectPtr conn,
> NULL)))
> goto cleanup;
>
> - if (!(dev = virDomainDeviceDefParse(xml, def, priv->caps, priv->xmlopt,
> - VIR_DOMAIN_DEF_PARSE_INACTIVE)))
> + if (!(dev = virDomainDeviceDefParse(xml, def, priv->caps,
> + priv->xmlopt, parse_flags);
> goto cleanup;
>
> if (virDomainXMLDevID(conn, minidef, dev, class, ref, sizeof(ref)))
> diff --git a/src/xen/xm_internal.c b/src/xen/xm_internal.c
> index ef1a460..ae142a3 100644
> --- a/src/xen/xm_internal.c
> +++ b/src/xen/xm_internal.c
> @@ -1322,6 +1322,8 @@ xenXMDomainDetachDeviceFlags(virConnectPtr conn,
> int ret = -1;
> size_t i;
> xenUnifiedPrivatePtr priv = conn->privateData;
> + unsigned int parse_flags = VIR_DOMAIN_DEF_PARSE_INACTIVE |
> + VIR_DOMAIN_DEF_PARSE_SKIP_GENERATE_MACADDR;
>
> virCheckFlags(VIR_DOMAIN_AFFECT_LIVE | VIR_DOMAIN_AFFECT_CONFIG, -1);
>
> @@ -1340,10 +1342,8 @@ xenXMDomainDetachDeviceFlags(virConnectPtr conn,
> goto cleanup;
> def = entry->def;
>
> - if (!(dev = virDomainDeviceDefParse(xml, entry->def,
> - priv->caps,
> - priv->xmlopt,
> - VIR_DOMAIN_DEF_PARSE_INACTIVE)))
> + if (!(dev = virDomainDeviceDefParse(xml, entry->def, priv->caps,
> + priv->xmlopt, parse_flags)))
> goto cleanup;
>
> switch (dev->type) {
>
> --
> libvir-list mailing list
> libvir-list@xxxxxxxxxx
> https://www.redhat.com/mailman/listinfo/libvir-list
>
--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list