Re: [libvirt] [PATCHv2] qemu: support live change of the bridge used by a guest network device

On 03/30/2012 06:25 AM, Daniel P. Berrange wrote: > On Fri, Mar 30, 2012 at 03:13:31AM -0400, Laine Stump wrote: >> On 03/29/2012 05:52 PM, Daniel P. Berrange wrote: >>> On Thu, Mar 29, 2012 at 04:38:50PM -0400, Laine Stump wrote: >>>> + if (oldbridge && >>>> + virNetDevBridgeRemovePort(oldbridge, olddev->ifname) < 0) { >>>> + return -1; >>>> + } >>>> + if (virNetDevBridgeAddPort(newbridge, olddev->ifname) < 0) { >>>> + if (virNetDevBridgeAddPort(oldbridge, olddev->ifname) < 0) { >>>> + qemuReportError(VIR_ERR_OPERATION_FAILED, >>>> + _("unable to recover former state by adding port" >>>> + "to bridge %s"), oldbridge); >>>> + } >>>> + return -1; >>>> + } >>> I think you need to emit 2 audit notifications here, one for the bridge >>> being removed and one for the bridge being added. >> That does sound like a good idea, but the current virDomainAuditNet() >> function only reports MAC address, and virDomainAuditNetDevice() only >> reports "/dev/net/tun" - neither of them gives any information about the >> name of tap device or which bridge it is being attached to. >> >> Right now, here are the audit messages that are logged when I do a full >> device detach/attach of a network device: >> >> type=VIRT_RESOURCE msg=audit(1333090567.694:1051): pid=0 uid=0 >> auid=4294967295 ses=4294967295 >> subj=system_u:system_r:virtd_t:s0-s0:c0.c1023 msg='virt=kvm resrc=net >> reason=detach vm="F14" uuid=de3aa186-be64-088e-b64a-a1a03e023ffd >> old-net=52:54:00:00:01:81 new-net=?: exe="/usr/sbin/libvirtd" hostname=? >> addr=? terminal=? res=success' >> >> type=VIRT_RESOURCE msg=audit(1333090573.195:1053): pid=0 uid=0 >> auid=4294967295 ses=4294967295 >> subj=system_u:system_r:virtd_t:s0-s0:c0.c1023 msg='virt=kvm resrc=net >> reason=open vm="F14" uuid=de3aa186-be64-088e-b64a-a1a03e023ffd >> net=52:54:00:00:01:81 path="/dev/net/tun" rdev=0A:C8: >> exe="/usr/sbin/libvirtd" hostname=? addr=? terminal=? res=success' >> type=VIRT_RESOURCE msg=audit(1333090573.196:1054): pid=0 uid=0 >> auid=4294967295 ses=4294967295 >> subj=system_u:system_r:virtd_t:s0-s0:c0.c1023 msg='virt=kvm resrc=net >> reason=open vm="F14" uuid=de3aa186-be64-088e-b64a-a1a03e023ffd >> net=52:54:00:00:01:81 path="/dev/vhost-net" rdev=0A:EE: >> exe="/usr/sbin/libvirtd" hostname=? addr=? terminal=? res=success' >> type=VIRT_RESOURCE msg=audit(1333090574.092:1055): pid=0 uid=0 >> auid=4294967295 ses=4294967295 >> subj=system_u:system_r:virtd_t:s0-s0:c0.c1023 msg='virt=kvm resrc=net >> reason=attach vm="F14" uuid=de3aa186-be64-088e-b64a-a1a03e023ffd >> old-net=? new-net=52:54:00:00:01:81: exe="/usr/sbin/libvirtd" hostname=? >> addr=? terminal=? res=success' >> >> It does a good job of telling me the MAC address that's going to be used >> by the domain, but nothing about how it's connected to the network. > Hmm, this seems flawed to me. The intent of the auditing code we added > was to provide information about any host resource that the VM is > associated with. So I'm really surprised we're not providing any info > about the host network interface. I suspect this should be fixed. > >> If we're staying within the current boundaries of reporting, is there >> really value in logging a pair of messages that are ultimately just >> telling us that the same mac address was detached then immediately >> reattached, but not saying anything about what it was connected to? >> Alternately, if we're going to start reporting about changes in network >> connection, shouldn't we also be reporting what those connections are to >> begin with? (I think that's a change in scope of what's being audited, >> and requires a separate patch if we decide we want to do that). > I think we should still audit this change, even though current audit > rules appear broken. Okay. Would the patch I've attached here be adequate? If so, I'll squash it into the rest of the patch. Beyond that, to fix the problem of general inadequacy in the current audits, would it be enough to add tap device and bridge names to the current attach and detach log messages? Or is the audit message format very strict, and we would need to do a separate log message for tap device and for bridge device?

From 3b277f2c2138b98145d6b5e18c844be5d43b8b29 Mon Sep 17 00:00:00 2001 From: Laine Stump <laine(a)laine.org&gt; Date: Fri, 30 Mar 2012 10:13:37 -0400 Subject: [PATCH] qemu: add audit logs when switching bridges This adds in a standard audit log for detaching and attaching a network device when the bridge being used is changed. The discussion about this led to the idea that the audit logs being generated are insufficient, since they don't say anything about which tap device is used, nor about which bridge it is attached to, but that should be fixed by a separate patch, and this gets the current patch properly wired into the infrastructure. --- src/qemu/qemu_hotplug.c | 8 +++++--- 1 files changed, 5 insertions(+), 3 deletions(-) diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c index 66837c4..7699e9d 100644 --- a/src/qemu/qemu_hotplug.c +++ b/src/qemu/qemu_hotplug.c @@ -1194,7 +1194,8 @@ static virDomainNetDefPtr qemuDomainFindNet(virDomainObjPtr vm, } static -int qemuDomainChangeNetBridge(virDomainNetDefPtr olddev, +int qemuDomainChangeNetBridge(virDomainObjPtr vm, + virDomainNetDefPtr olddev, virDomainNetDefPtr newdev) { const char *oldbridge = olddev->data.bridge.brname; @@ -1221,9 +1222,11 @@ int qemuDomainChangeNetBridge(virDomainNetDefPtr olddev, } return -1; } + virDomainAuditNet(vm, NULL, olddev, "detach", true);

VIR_FREE(olddev->data.bridge.brname); olddev->data.bridge.brname = newdev->data.bridge.brname; newdev->data.bridge.brname = NULL; + virDomainAuditNet(vm, NULL, olddev, "attach", true); return 0; }