On 03/30/2012 10:39 AM, Daniel P. Berrange wrote:
> On Fri, Mar 30, 2012 at 10:30:01AM -0400, Laine Stump wrote:
>> From 3b277f2c2138b98145d6b5e18c844be5d43b8b29 Mon Sep 17 00:00:00 2001
>> From: Laine Stump <la...@laine.org>
>> 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);
> This needs to be moved up, otherwise if we successfully detach, but
> fail to attach we'll not emit the audit message. Also we I think
> we should log "detach", false if detaching fails, and likewise
> 'attach', false' if attaching fails
>
Okay, I've redone the auditing calls, placing them immediately after
each call to add/remove port, calling regardless of whether the
operation was successful or not. I haven't addressed the higher level
problem of adding bridge info to the audit message (although I have made
sure that the NetDef object sent to the audit function has the proper
information - that's why the brname is moved around at different times
than previously).
Does this (squashed into the initial patch that had no auditing) look
adequate?
(I've tested this code, including failure to attach to the new bridge
(by trying to attach to "eth0"), and it does work as you suggest).
>From 7fc635f4234963abad95b66f29bcaa7d02d78de2 Mon Sep 17 00:00:00 2001
From: Laine Stump <la...@laine.org>
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.
All *attempts* to detach or attach a tap to a bridge are logged, along
with whether or not they are successful.
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 | 36 ++++++++++++++++++++++++------------
1 files changed, 24 insertions(+), 12 deletions(-)
diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c
index ce7921c..80b53d7 100644
--- a/src/qemu/qemu_hotplug.c
+++ b/src/qemu/qemu_hotplug.c
@@ -1194,11 +1194,13 @@ 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;
- const char *newbridge = newdev->data.bridge.brname;
+ int ret = -1;
+ char *oldbridge = olddev->data.bridge.brname;
+ char *newbridge = newdev->data.bridge.brname;
VIR_DEBUG("Change bridge for interface %s: %s -> %s",
olddev->ifname, oldbridge, newbridge);
@@ -1209,20 +1211,31 @@ int qemuDomainChangeNetBridge(virDomainNetDefPtr olddev,
return -1;
}
- if (oldbridge &&
- virNetDevBridgeRemovePort(oldbridge, olddev->ifname) < 0) {
- return -1;
+ if (oldbridge) {
+ ret = virNetDevBridgeRemovePort(oldbridge, olddev->ifname);
+ virDomainAuditNet(vm, olddev, NULL, "detach", ret == 0);
+ if (ret < 0)
+ return ret;
}
- if (virNetDevBridgeAddPort(newbridge, olddev->ifname) < 0) {
- if (virNetDevBridgeAddPort(oldbridge, olddev->ifname) < 0) {
+
+ /* move newbridge into olddev now so Audit log is correct */
+ olddev->data.bridge.brname = newbridge;
+ ret = virNetDevBridgeAddPort(newbridge, olddev->ifname);
+ virDomainAuditNet(vm, NULL, olddev, "attach", ret == 0);
+ if (ret < 0) {
+ /* restore oldbridge to olddev */
+ olddev->data.bridge.brname = oldbridge;
+ ret = virNetDevBridgeAddPort(oldbridge, olddev->ifname);
+ virDomainAuditNet(vm, NULL, olddev, "attach", ret == 0);
+ if (ret < 0) {
qemuReportError(VIR_ERR_OPERATION_FAILED,
_("unable to recover former state by adding port"
"to bridge %s"), oldbridge);
}
return -1;
}
- VIR_FREE(olddev->data.bridge.brname);
- olddev->data.bridge.brname = newdev->data.bridge.brname;
+ /* oldbridge no longer needed, and newbridge moved to olddev */
+ VIR_FREE(oldbridge);
newdev->data.bridge.brname = NULL;
return 0;
}
@@ -1368,10 +1381,9 @@ int qemuDomainChangeNet(struct qemud_driver *driver,
}
if (olddev->type == VIR_DOMAIN_NET_TYPE_BRIDGE
- && dev->type == VIR_DOMAIN_NET_TYPE_BRIDGE
&& STRNEQ_NULLABLE(olddev->data.bridge.brname,
dev->data.bridge.brname)) {
- if ((ret = qemuDomainChangeNetBridge(olddev, dev)) < 0)
+ if ((ret = qemuDomainChangeNetBridge(vm, olddev, dev)) < 0)
return ret;
}
--
1.7.7.6
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
