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

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.