On Fri, Oct 02, 2009 at 03:48:36PM +0200, Gerhard Stenzel wrote:
> This patch adds MAC address based port filtering to the qemu driver.
>
> Signed-off-by: Gerhard Stenzel <gerhard.stenzel@xxxxxxxxxx>
> ---
>
> src/qemu/qemu.conf | 3 +++
> src/qemu/qemu_conf.c | 14 ++++++++++++++
> src/qemu/qemu_conf.h | 2 ++
> src/qemu/qemu_driver.c | 23 +++++++++++++++++++++++
> 4 files changed, 42 insertions(+), 0 deletions(-)
> @@ -1193,6 +1197,16 @@ qemudNetworkIfaceConnect(virConnectPtr conn,
> tapfd = -1;
> }
>
> + if (driver->macFilter) {
> + virNetworkPtr network = virNetworkLookupByName(conn,
> + net->data.network.name);
> + if ((err = virNetworkAllowMacOnPort(network, brname, net->ifname, net->mac))) {
> + virReportSystemError(conn, err,
> + _("failed to add ebtables rule to allow MAC address on '%s'"),
> + net->ifname);
> + }
> + }
This will crash in a large number of scenarios, since it is
only valid to deference net->data.network fields once you
have verified net->type == VIR_DOMAIN_NET_TYPE_NETWORK. It
is also failing to check for virNetworkLookupByName() returning
NULL.
This is why the MAC filtering should not be part of the
virNetwork API set. The QEMU driver should be directly
calling the ebtables APIs you added in patch 1, rather
then indirectly via virNetwork.. This would allow this
MAC filtering to work with bridged network modes too.
> diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
> index 155e4a3..a95c867 100644
> --- a/src/qemu/qemu_driver.c
> +++ b/src/qemu/qemu_driver.c
> @@ -239,6 +239,14 @@ qemudAutostartConfigs(struct qemud_driver *driver) {
> }
> virDomainObjUnlock(vm);
> }
> + if (qemu_driver->macFilter) {
> + fprintf(stderr,"%s:%s:%d macFilter=%d\n", __FILE__, __FUNCTION__, __LINE__, qemu_driver->macFilter);
There's a VIR_DEBUG() macro available for logging.
> + if ((errno = virNetworkDisableAllFrames(conn))) {
> + virReportSystemError(conn, errno,
> + _("failed to add rule to drop all frames in '%s'"), __FILE__);
> + }
> + }
> +
> qemuDriverUnlock(driver);
>
> if (conn)
> @@ -2167,8 +2175,23 @@ cleanup:
> static void qemudShutdownVMDaemon(virConnectPtr conn,
> struct qemud_driver *driver,
> virDomainObjPtr vm) {
> +
> int ret;
> int retries = 0;
> + char *brname;
> +
> + virDomainNetDefPtr net = vm->def->nets[0];
This assumes the guest has exactly one NIC - it'll crash if there
are no NICs, and it'll miss cleanup steps if there are multiple NICs
> + virNetworkPtr network = virNetworkLookupByName(conn,
> + net->data.network.name);
> + brname = virNetworkGetBridgeName(network);
> +
> + if (driver->macFilter) {
> + if ((errno = virNetworkDisallowMacOnPort(network, brname, net->ifname, net->mac))) {
> + virReportSystemError(conn, errno,
> + _("failed to add ebtables rule to allow MAC address on '%s'"),
> + net->ifname);
> + }
> + }
Same comment as before about not using virNetwork for any of this
Regards,
Daniel
--
|: Red Hat, Engineering, London -o- http://people.redhat.com/berrange/ :|
|: http://libvirt.org -o- http://virt-manager.org -o- http://ovirt.org :|
|: http://autobuild.org -o- http://search.cpan.org/~danberr/ :|
|: GnuPG: 7D3B9505 -o- F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 :|
--
Libvir-list mailing list
Libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list