Re: [PATCH 4/6] qemu: qemuMonitorQueryRxFilter - retrieve guest netdev rx-filter

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 




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

--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list




[Index of Archives]     [Virt Tools]     [Libvirt Users]     [Lib OS Info]     [Fedora Users]     [Fedora Desktop]     [Fedora SELinux]     [Big List of Linux Books]     [Yosemite News]     [KDE Users]     [Fedora Tools]