[Qemu-devel] Re: [PATCH 02/22] net: replace qemu_nic_format_info_str and VLANClientState->info_str by QDicts

[Luiz Capitulino]

On Thu,  8 Apr 2010 17:16:19 -0300
Miguel Di Ciurcio Filho <address@hidden> wrote:

> There is no standard format when formatting info_str, so it is difficult
> to extract information and transmit it over QMP.
> 
> This patch changes info_str from a simple string to a QDict.
> 
> Patches that convert the devices to this new format will follow.

 This will break the build of each individual patch, which will break
git bisect. Each individual patch should build when applied.

 To fix that, you can add the new function and only drop the current one
when the full conversion is done. The temporary duplication is ok.

 More comments below.

> Signed-off-by: Miguel Di Ciurcio Filho <address@hidden>
> ---
>  net.c |   14 ++++++++++----
>  net.h |    4 ++--
>  2 files changed, 12 insertions(+), 6 deletions(-)
> 
> diff --git a/net.c b/net.c
> index 3ede738..ad01732 100644
> --- a/net.c
> +++ b/net.c
> @@ -35,6 +35,8 @@
>  #include "sysemu.h"
>  #include "qemu-common.h"
>  #include "qemu_socket.h"
> +#include "qdict.h"
> +#include "qstring.h"
>  #include "hw/qdev.h"
>  
>  static QTAILQ_HEAD(, VLANState) vlans;
> @@ -164,13 +166,17 @@ int parse_host_port(struct sockaddr_in *saddr, const 
> char *str)
>      return 0;
>  }
>  
> -void qemu_format_nic_info_str(VLANClientState *vc, uint8_t macaddr[6])
> +void qemu_format_nic_info_dict(VLANClientState *vc, uint8_t macaddr[6])
>  {
> -    snprintf(vc->info_str, sizeof(vc->info_str),
> -             "model=%s,macaddr=%02x:%02x:%02x:%02x:%02x:%02x",
> -             vc->model,
> +    vc->info_dict = qdict_new();
> +    char mac[18];
> +
> +    snprintf(mac, 18, "%02x:%02x:%02x:%02x:%02x:%02x",
>               macaddr[0], macaddr[1], macaddr[2],
>               macaddr[3], macaddr[4], macaddr[5]);

 sizeof(mac) is better.

> +
> +    qdict_put(vc->info_dict, "macaddr", qstring_from_str(mac));
> +    qdict_put(vc->info_dict, "model", qstring_from_str(vc->model));
>  }
>  
>  void qemu_macaddr_default_if_unset(MACAddr *macaddr)
> diff --git a/net.h b/net.h
> index 16f19c5..b744294 100644
> --- a/net.h
> +++ b/net.h
> @@ -65,7 +65,7 @@ struct VLANClientState {
>      NetQueue *send_queue;
>      char *model;
>      char *name;
> -    char info_str[256];
> +    QDict *info_dict;

 Isn't VLANClientState freed somewhere? At least in netdev_del it should.. So,
you have to do a QDECREF(info_dict) there.

>      unsigned receive_disabled : 1;
>  };
>  
> @@ -110,7 +110,7 @@ ssize_t qemu_send_packet_async(VLANClientState *vc, const 
> uint8_t *buf,
>                                 int size, NetPacketSent *sent_cb);
>  void qemu_purge_queued_packets(VLANClientState *vc);
>  void qemu_flush_queued_packets(VLANClientState *vc);
> -void qemu_format_nic_info_str(VLANClientState *vc, uint8_t macaddr[6]);
> +void qemu_format_nic_info_dict(VLANClientState *vc, uint8_t macaddr[6]);
>  void qemu_macaddr_default_if_unset(MACAddr *macaddr);
>  int qemu_show_nic_models(const char *arg, const char *const *models);
>  void qemu_check_nic_model(NICInfo *nd, const char *model);