[libvirt] [PATCH 4/6] qemu: qemuMonitorQueryRxFilter - retrieve guest netdev rx-filter
John Ferlan
jferlan at redhat.com
Thu Sep 25 00:11:37 UTC 2014
On 09/24/2014 05:50 AM, Laine Stump wrote:
> This function can be called at any time to get the current status of a
> guest's network device rx-filter. In particular it is useful to call
> after libvirt recieves a NIC_RX_FILTER_CHANGED event - this event only
> tells you that something has changed in the rx-filter, the details are
> retrieved with the query-rx-filter monitor command (only available in
> the json monitor). The commend sent to the qemu monitor looks like this:
>
> {"execute":"query-rx-filter", "arguments": {"name":"net2"} }'
>
> and the results will look something like this:
>
> {
> "return": [
> {
> "promiscuous": false,
> "name": "net2",
> "main-mac": "52:54:00:98:2d:e3",
> "unicast": "normal",
> "vlan": "normal",
> "vlan-table": [
> 42,
> 0
> ],
> "unicast-table": [
>
> ],
> "multicast": "normal",
> "multicast-overflow": false,
> "unicast-overflow": false,
> "multicast-table": [
> "33:33:ff:98:2d:e3",
> "01:80:c2:00:00:21",
> "01:00:5e:00:00:fb",
> "33:33:ff:98:2d:e2",
> "01:00:5e:00:00:01",
> "33:33:00:00:00:01"
> ],
> "broadcast-allowed": false
> }
> ],
> "id": "libvirt-14"
> }
>
> This is all parsed from JSON into a virNetDevRxFilter object for
> easier consumption. (unicast-table is usually empty, but is also an
> array of mac addresses similar to multicast-table).
>
> (NB: LIBNL_CFLAGS was added to tests/Makefile.am because virnetdev.h
> now includes util/virnetlink.h, which includes netlink/msg.h when
> appropriate. Without LIBNL_CFLAGS, gcc can't find that file (if
> libnl/netlink isn't available, LIBNL_CFLAGS will be empty and
> virnetlink.h won't try to include netlink/msg.h anyway).)
> ---
> src/qemu/qemu_monitor.c | 26 ++++++
> src/qemu/qemu_monitor.h | 4 +
> src/qemu/qemu_monitor_json.c | 215 +++++++++++++++++++++++++++++++++++++++++++
> src/qemu/qemu_monitor_json.h | 3 +
> tests/Makefile.am | 3 +
> 5 files changed, 251 insertions(+)
>
> diff --git a/src/qemu/qemu_monitor.c b/src/qemu/qemu_monitor.c
> index c25f002..48cbe3e 100644
> --- a/src/qemu/qemu_monitor.c
> +++ b/src/qemu/qemu_monitor.c
> @@ -2918,6 +2918,32 @@ int qemuMonitorRemoveNetdev(qemuMonitorPtr mon,
> }
>
>
> +int
> +qemuMonitorQueryRxFilter(qemuMonitorPtr mon, const char *alias,
> + virNetDevRxFilterPtr *filter)
> +{
> + int ret = -1;
> + VIR_DEBUG("mon=%p alias=%s filter=%p",
> + mon, alias, filter);
> +
> + if (!mon) {
> + virReportError(VIR_ERR_INVALID_ARG, "%s",
> + _("monitor must not be NULL"));
> + return -1;
> + }
The "if (!mon->json)" usually goes here rather than later as an else.
Just trying to be consistent with other functions
I'm assuming at some point whomever calls this will check if the
query-rx-filter command exits (eg, the qemu capability exists)...
Again I haven't peeked ahead.
> +
> +
> + VIR_DEBUG("mon=%p, alias=%s", mon, alias);
> +
> + if (mon->json)
> + ret = qemuMonitorJSONQueryRxFilter(mon, alias, filter);
> + else
> + virReportError(VIR_ERR_OPERATION_UNSUPPORTED, "%s",
> + _("query-rx-filter requires JSON monitor"));
> + return ret;
> +}
> +
> +
> int qemuMonitorGetPtyPaths(qemuMonitorPtr mon,
> virHashTablePtr paths)
> {
> diff --git a/src/qemu/qemu_monitor.h b/src/qemu/qemu_monitor.h
> index 6b91e29..c37e36f 100644
> --- a/src/qemu/qemu_monitor.h
> +++ b/src/qemu/qemu_monitor.h
> @@ -31,6 +31,7 @@
> # include "virbitmap.h"
> # include "virhash.h"
> # include "virjson.h"
> +# include "virnetdev.h"
> # include "device_conf.h"
> # include "cpu/cpu.h"
>
> @@ -622,6 +623,9 @@ int qemuMonitorAddNetdev(qemuMonitorPtr mon,
> int qemuMonitorRemoveNetdev(qemuMonitorPtr mon,
> const char *alias);
>
> +int qemuMonitorQueryRxFilter(qemuMonitorPtr mon, const char *alias,
> + virNetDevRxFilterPtr *filter);
> +
> int qemuMonitorGetPtyPaths(qemuMonitorPtr mon,
> virHashTablePtr paths);
>
> diff --git a/src/qemu/qemu_monitor_json.c b/src/qemu/qemu_monitor_json.c
> index a3d7c2c..58007e6 100644
> --- a/src/qemu/qemu_monitor_json.c
> +++ b/src/qemu/qemu_monitor_json.c
> @@ -3194,6 +3194,221 @@ int qemuMonitorJSONRemoveNetdev(qemuMonitorPtr mon,
> }
>
>
> +static int
> +qemuMonitorJSONQueryRxFilterParse(virJSONValuePtr msg,
> + virNetDevRxFilterPtr *filter)
> +{
> + int ret = -1;
> + const char *tmp;
> + virJSONValuePtr returnArray, entry, table, element;
> + int nTable;
> + size_t i;
> + virNetDevRxFilterPtr fil = virNetDevRxFilterNew();
> +
> + if (!(returnArray = virJSONValueObjectGet(msg, "return"))) {
> + virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
> + _("query-rx-filter reply was missing return data"));
> + goto cleanup;
> + }
> + if (returnArray->type != VIR_JSON_TYPE_ARRAY) {
> + virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
> + _("query-rx-filter return data was not an array"));
> + goto cleanup;
> + }
> + if (!(entry = virJSONValueArrayGet(returnArray, 0))) {
> + virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
> + _("query -rx-filter returne data missing array element"));
> + goto cleanup;
> + }
> +
> + if (!fil)
> + goto cleanup;
> +
> + if (!(tmp = virJSONValueObjectGetString(entry, "name"))) {
> + virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
> + _("Missing or invalid name "
> + "in query-rx-filter response"));
> + goto cleanup;
> + }
> + if (VIR_STRDUP(fil->name, tmp) < 0)
> + goto cleanup;
> + if ((!(tmp = virJSONValueObjectGetString(entry, "main-mac"))) ||
> + virMacAddrParse(tmp, &fil->mac) < 0) {
> + virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
> + _("Missing or invalid 'main-mac' "
> + "in query-rx-filter response"));
> + goto cleanup;
> + }
> + if (virJSONValueObjectGetBoolean(entry, "promiscuous",
> + &fil->promiscuous) < 0) {
> + virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
> + _("Missing or invalid 'promiscuous' "
> + "in query-rx-filter response"));
> + goto cleanup;
> + }
> + if (virJSONValueObjectGetBoolean(entry, "broadcast-allowed",
> + &fil->broadcastAllowed) < 0) {
> + virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
> + _("Missing or invalid 'broadcast-allowed' "
> + "in query-rx-filter response"));
> + goto cleanup;
> + }
> +
> + if ((!(tmp = virJSONValueObjectGetString(entry, "unicast"))) ||
> + ((fil->unicast.mode
> + = virNetDevRxFilterUnicastModeTypeFromString(tmp)) < 0)) {
Unless different values for each could be returned - I think the common
function would work - requires change here...
> + virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
> + _("Missing or invalid 'unicast' "
> + "in query-rx-filter response"));
> + goto cleanup;
> + }
> + if (virJSONValueObjectGetBoolean(entry, "unicast-overflow",
> + &fil->unicast.overflow) < 0) {
> + virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
> + _("Missing or invalid 'unicast-overflow' "
> + "in query-rx-filter response"));
> + goto cleanup;
> + }
> + if ((!(table = virJSONValueObjectGet(entry, "unicast-table"))) ||
> + ((nTable = virJSONValueArraySize(table)) < 0)) {
> + virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
> + _("Missing or invalid 'unicast-table' array "
> + "in query-rx-filter response"));
> + goto cleanup;
> + }
> + if (VIR_ALLOC_N(fil->unicast.table, nTable))
> + goto cleanup;
> + for (i = 0; i < nTable; i++) {
> + if (!(element = virJSONValueArrayGet(table, i)) ||
> + !(tmp = virJSONValueGetString(element))) {
> + virReportError(VIR_ERR_INTERNAL_ERROR,
> + _("Missing or invalid element %zu of 'unicast' "
> + "list in query-rx-filter response"), i);
> + goto cleanup;
> + }
> + if (virMacAddrParse(tmp, &fil->unicast.table[i]) < 0) {
> + virReportError(VIR_ERR_INTERNAL_ERROR,
> + _("invalid mac address '%s' in 'unicast-table' "
> + "array in query-rx-filter response"), tmp);
> + goto cleanup;
> + }
> + }
> + fil->unicast.nTable = nTable;
hmm.. so this is what relates to patch 3's query... Looks like a [n]
entry table of virMacAddr where each entry just gets copied in place -
so the single VIR_FREE should be fine.
No issue - just thinking outloud to help me make a better ACK for patch3
> +
> + if ((!(tmp = virJSONValueObjectGetString(entry, "multicast"))) ||
> + ((fil->multicast.mode
> + = virNetDevRxFilterMulticastModeTypeFromString(tmp)) < 0)) {
Unless different values for each could be returned - I think the common
function would work - requires change here...
> + virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
> + _("Missing or invalid 'multicast' "
> + "in query-rx-filter response"));
> + goto cleanup;
> + }
> + if (virJSONValueObjectGetBoolean(entry, "multicast-overflow",
> + &fil->multicast.overflow) < 0) {
> + virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
> + _("Missing or invalid 'multicast-overflow' "
> + "in query-rx-filter response"));
> + goto cleanup;
> + }
> + if ((!(table = virJSONValueObjectGet(entry, "multicast-table"))) ||
> + ((nTable = virJSONValueArraySize(table)) < 0)) {
> + virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
> + _("Missing or invalid 'multicast-table' array "
> + "in query-rx-filter response"));
> + goto cleanup;
> + }
> + if (VIR_ALLOC_N(fil->multicast.table, nTable))
> + goto cleanup;
> + for (i = 0; i < nTable; i++) {
> + if (!(element = virJSONValueArrayGet(table, i)) ||
> + !(tmp = virJSONValueGetString(element))) {
> + virReportError(VIR_ERR_INTERNAL_ERROR,
> + _("Missing or invalid element %zu of 'multicast' "
> + "list in query-rx-filter response"), i);
> + goto cleanup;
> + }
> + if (virMacAddrParse(tmp, &fil->multicast.table[i]) < 0) {
> + virReportError(VIR_ERR_INTERNAL_ERROR,
> + _("invalid mac address '%s' in 'multicast-table' "
> + "array in query-rx-filter response"), tmp);
> + goto cleanup;
> + }
> + }
> + fil->multicast.nTable = nTable;
> +
> + if ((!(tmp = virJSONValueObjectGetString(entry, "vlan"))) ||
> + ((fil->vlan.mode
> + = virNetDevRxFilterVlanModeTypeFromString(tmp)) < 0)) {
Unless different values for each could be returned - I think the common
function would work - requires change here...
> + virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
> + _("Missing or invalid 'vlan' "
> + "in query-rx-filter response"));
> + goto cleanup;
> + }
> + if ((!(table = virJSONValueObjectGet(entry, "vlan-table"))) ||
> + ((nTable = virJSONValueArraySize(table)) < 0)) {
> + virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
> + _("Missing or invalid 'vlan-table' array "
> + "in query-rx-filter response"));
> + goto cleanup;
> + }
> + if (VIR_ALLOC_N(fil->vlan.table, nTable))
> + goto cleanup;
> + for (i = 0; i < nTable; i++) {
> + if (!(element = virJSONValueArrayGet(table, i)) ||
> + virJSONValueGetNumberUint(element, &fil->vlan.table[i]) < 0) {
> + virReportError(VIR_ERR_INTERNAL_ERROR,
> + _("Missing or invalid element %zu of 'vlan-table' "
> + "array in query-rx-filter response"), i);
> + goto cleanup;
> + }
> + }
> + fil->vlan.nTable = nTable;
> +
> + ret = 0;
> + cleanup:
> + if (ret < 0) {
> + virNetDevRxFilterFree(fil);
> + fil = NULL;
> + }
> + *filter = fil;
> + return ret;
> +}
> +
> +
> +int
> +qemuMonitorJSONQueryRxFilter(qemuMonitorPtr mon, const char *alias,
> + virNetDevRxFilterPtr *filter)
> +{
> + int ret = -1;
> + virJSONValuePtr cmd = qemuMonitorJSONMakeCommand("query-rx-filter",
> + "s:name", alias,
> + NULL);
> + virJSONValuePtr reply = NULL;
> +
> + if (!cmd)
> + goto cleanup;
> +
> + if (qemuMonitorJSONCommand(mon, cmd, &reply) < 0)
> + goto cleanup;
> +
> + if (qemuMonitorJSONQueryRxFilterParse(reply, filter) < 0)
> + goto cleanup;
> +
> + ret = 0;
> + cleanup:
> + if (ret == 0)
> + ret = qemuMonitorJSONCheckError(cmd, reply);
> +
> + if (ret < 0) {
> + virNetDevRxFilterFree(*filter);
> + *filter = NULL;
> + }
> + virJSONValueFree(cmd);
> + virJSONValueFree(reply);
> + return ret;
> +}
> +
> +
> /*
> * Example return data
> *
> diff --git a/src/qemu/qemu_monitor_json.h b/src/qemu/qemu_monitor_json.h
> index d366179..a41281b 100644
> --- a/src/qemu/qemu_monitor_json.h
> +++ b/src/qemu/qemu_monitor_json.h
> @@ -210,6 +210,9 @@ int qemuMonitorJSONAddNetdev(qemuMonitorPtr mon,
> int qemuMonitorJSONRemoveNetdev(qemuMonitorPtr mon,
> const char *alias);
>
> +int qemuMonitorJSONQueryRxFilter(qemuMonitorPtr mon, const char *alias,
> + virNetDevRxFilterPtr *filter);
> +
> int qemuMonitorJSONGetPtyPaths(qemuMonitorPtr mon,
> virHashTablePtr paths);
>
> diff --git a/tests/Makefile.am b/tests/Makefile.am
> index d6c3cfb..bd4371b 100644
> --- a/tests/Makefile.am
> +++ b/tests/Makefile.am
> @@ -35,6 +35,7 @@ AM_CFLAGS = \
> -Dabs_builddir="\"$(abs_builddir)\"" \
> -Dabs_srcdir="\"$(abs_srcdir)\"" \
> $(LIBXML_CFLAGS) \
> + $(LIBNL_CFLAGS) \
> $(GNUTLS_CFLAGS) \
> $(SASL_CFLAGS) \
> $(SELINUX_CFLAGS) \
> @@ -43,6 +44,8 @@ AM_CFLAGS = \
> $(COVERAGE_CFLAGS) \
> $(WARN_CFLAGS)
>
> +
> +
Not sure why there's extra lines here? Should be removed
> AM_LDFLAGS = \
> -export-dynamic
>
>
Seeing libnl copied into the Makefile just sends up the warning flag in
general about libnl
ACK (maybe Eric has a different thought about the Makefile)
John
More information about the libvir-list
mailing list