Re: [libvirt] [PATCH 6/6] qemu: change macvtap device MAC address in response to NIC_RX_FILTER_CHANGED
On 09/24/2014 10:19 PM, John Ferlan wrote:
On 09/24/2014 05:50 AM, Laine Stump wrote:
> This patch fills in the functionality of
> processNicRxFilterChangedEvent(). It now checks if it is appropriate
> to respond to the NIC_RX_FILTER_CHANGED event (based on device type
> and configuration) and takes appropriate action. Currently it checks
> if the guest interface has been configured with
> trustGuestRxFilters='yes', and if the host side device is macvtap. If
> so, and the MAC address on the guest has changed, the MAC address of
> the macvtap device is changed to match.
>
> The result of this is that networking from the guest will continue to
> work if the mac address of a macvtap-connected network device is
> changed from within the guest, as long as trustGuestRxFilters='yes'
> (previously changing the MAC address in the guest would break
> networking).
> ---
> src/qemu/qemu_driver.c | 50 ++++++++++++++++++++++++++++++++++++++++++++++++++
> 1 file changed, 50 insertions(+)
>
> diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
> index 64f1d45..7801d91 100644
> --- a/src/qemu/qemu_driver.c
> +++ b/src/qemu/qemu_driver.c
> @@ -4146,8 +4146,13 @@ processNicRxFilterChangedEvent(virQEMUDriverPtr driver,
> char *devAlias)
> {
> virQEMUDriverConfigPtr cfg = virQEMUDriverGetConfig(driver);
> + qemuDomainObjPrivatePtr priv = vm->privateData;
> virDomainDeviceDef dev;
> virDomainNetDefPtr def;
> + virNetDevRxFilterPtr filter = NULL;
> + virMacAddr oldMAC;
> + char newMacStr[VIR_MAC_STRING_BUFLEN];
> + int ret;
>
> VIR_DEBUG("Received NIC_RX_FILTER_CHANGED event for device %s "
> "from domain %p %s",
> @@ -4175,11 +4180,55 @@ processNicRxFilterChangedEvent(virQEMUDriverPtr driver,
> }
> def = dev.data.net;
>
> + if (!virDomainNetGetActualTrustGuestRxFilters(def)) {
> + VIR_WARN("ignore NIC_RX_FILTER_CHANGED event for network "
> + "device %s in domain %s",
> + def->info.alias, vm->def->name);
So how often could this be emitted? Do we need some sort of "filter" to
only message once per life of the domain?
As often as we respond to the previous NIC_RX_FILTER_CHANGED event -
qemu won't send another event until we've issued a new query-rx-filter
command for that same interface. So for domains where we've set
trustGuestRxFilters='no', we will get exactly one event per interface.
On domains where we trust the guest enough to use its filter info, we
will never get a backlog of more than one per interface.
> + /* not sending "query-rx-filter" will also suppress any
> + * further NIC_RX_FILTER_CHANGED events for this device
> + */
> + goto endjob;
> + }
> +
> /* handle the event - send query-rx-filter and respond to it. */
>
> VIR_DEBUG("process NIC_RX_FILTER_CHANGED event for network "
> "device %s in domain %s", def->info.alias,
vm->def->name);
>
There's no capabilities check necessary? What if the command doesn't
exist in the underlying qemu?
If NIC_RX_FILTER_CHANGED exists, then query-rx-filter exists. If
NIC_RX_FILTER_CHANGED doesn't exist, we will never get to this point.
> + qemuDomainObjEnterMonitor(driver, vm);
> + ret = qemuMonitorQueryRxFilter(priv->mon, devAlias, &filter);
> + qemuDomainObjExitMonitor(driver, vm);
> + if (ret < 0)
> + goto endjob;
You filled in a lot of data from the qemuMonitorQueryRxFilter(), but
you're only using filter->mac (e.g. main-mac).
Or am I missing something?
No, you're not missing anything. All the other information will be used
in the future in a similar manner. This single patch was all that was
necessary to get regular unicast traffic working after a mac address
change, so it is useful on its own, but I didn't want to add in the
utility functions half-finished.