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@linux.vnet.ibm.com> --- 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@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list