On 11/03/2010 06:43 AM, Daniel P. Berrange wrote:
On Tue, Nov 02, 2010 at 07:04:01PM -0400, Stefan Berger wrote:
On 11/02/2010 11:47 AM, Daniel P. Berrange wrote:
On Tue, Nov 02, 2010 at 11:35:44AM -0400, Stefan Berger wrote:
During a shutdown/restart cycle libvirtd forgot the macvtap device name
that it had created on behalf of a VM so that a stale macvtap device
remained on the host when the VM terminated. Libvirtd has to actively
tear down a macvtap device and it uses its name for identifying which
device to tear down.
The solution is to not blank out the<target dev='...'/> completely, but
only blank it out on VMs that are not active. So, if a VM is active, the
device name makes it into the XML and is also being parsed. If a VM is
not active, the device name is discarded.
Signed-off-by: Stefan Berger<stefanb@xxxxxxxxxx>
---
src/conf/domain_conf.c | 5 ++++-
1 file changed, 4 insertions(+), 1 deletion(-)
Index: libvirt-acl/src/conf/domain_conf.c
===================================================================
--- libvirt-acl.orig/src/conf/domain_conf.c
+++ libvirt-acl/src/conf/domain_conf.c
@@ -2343,7 +2343,8 @@ virDomainNetDefParseXML(virCapsPtr caps,
def->data.direct.linkdev = dev;
dev = NULL;
- VIR_FREE(ifname);
+ if ((flags& VIR_DOMAIN_XML_INACTIVE))
+ VIR_FREE(ifname);
The conditional isn't required here - it is already dealt
with earlier on in the file. Just remove the VIR_FREE
completely.
The conditional further above is this code fragment here:
} else if ((ifname == NULL)&&
xmlStrEqual(cur->name, BAD_CAST "target")) {
ifname = virXMLPropString(cur, "dev");
if ((ifname != NULL)&&
((flags& VIR_DOMAIN_XML_INACTIVE)&&
(STRPREFIX((const char*)ifname, "vnet")))) {
/* An auto-generated target name, blank it out */
VIR_FREE(ifname);
}
Unfortunately it is also testing for the prefix 'vnet'.
In case of a macvtap device I don't want to pick up the name of the
macvtap device from the XML unless it's attached to a running domain. So
that's why I remove it above.
If the domain went down without libvirt 'seeing' it then we miessed out
on tearing it down and in that case there may be a stale device, but I
don't support this case. So I also want to have that cleared.
Further, I also don't accept user-provided interface names for the
reason of tear-down in case of failures while trying to start a VM. In
the failure-case it is not clear anymore whether the name was
user-provided and was previously created and needs to be torn down or
simply is a user-provided name of an interface that wasn't created and
thus should not be torn down because it may actually be clashing with
the user-provided name of a running VM. I had that before and this ended
up running danger of tearing down interfaces of running VMs when a
failure-path was invoked.
Hmm, I didn't notice that macvtap didn't allow user provided interface
names. IMHO this is a bug, because many users like to have predetermined
names for the devices to allow easy matching up to VMs.
The problem scenario you describe here with teardown killing the
interface of another running guests, only occurs if the admin has
configured two guests with the same macvtap device name. This is
user error. We're punishing everybody by disallowing user provided
names, just in case a minority create a broken config :-(
I think in case of macvtap this would create more security holes than
provide a benefit. To be safe that the name of an interface is not
already used, we'd have to cross-check with all other domains'
interfaces, otherwise people will wonder what happened when a VM
couldn't start and now another VM doesn't have its interface anymore.
People do forget what names they provided to interfaces. Can multiple
users define domains on the same machine and create those clashes? If
there was an elegant solution, I'd like to support it, but for now
fixing the libvirtd's forgetfullness is more important.. also since it's
just a netto one-liner...
Stefan
IMHO we should make this work as normal tap devices do. vnet* is
a prefix for auto-generated names. Anything else is a user provided
name.
Regards,
Daniel
--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list