Re: [PATCH v3 01/46] net: add qemu_{configure, create}_nic_device(), qemu_find_nic_info()

[David Woodhouse]

On Fri, 2024-01-26 at 12:10 +0100, Thomas Huth wrote:
> 
> > +/* "Please create a device, if you have a configuration for it" */
> > +DeviceState *qemu_create_nic_device(const char *typename, bool 
> > match_default,
> > +                                    const char *alias)
> > +{
> > +    NICInfo *nd = qemu_find_nic_info(typename, match_default, alias);
> > +    DeviceState *dev;
> > +
> > +    if (!nd) {
> > +        return NULL;
> > +    }
> 
> The qemu_check_nic_model() function that was used in some code that you 
> turned into qemu_create_nic_device() used to set:
> 
>      if (!nd->model)
>          nd->model = g_strdup(default_model);
> 
> (in the qemu_find_nic_model() function that has been called by 
> qemu_check_nic_model())
> 
> Should we do that also here to make sure that nd->model is not NULL 
> afterwards?

Good question, but I don't think we care. The qdev_set_nic_properties()
function certainly doesn't propagate nd->model to anywhere.

I renamed nd->model to nd->modelname in a patch shown below, just to be
100% sure I'm not missing any other code paths which might consume it.


diff --git a/include/net/net.h b/include/net/net.h
index 766201c62c..ad6cd5b14b 100644
--- a/include/net/net.h
+++ b/include/net/net.h
@@ -238,7 +238,7 @@ GPtrArray *qemu_get_nic_models(const char *device_type);
 
 struct NICInfo {
     MACAddr macaddr;
-    char *model;
+    char *modelname;
     char *name;
     char *devaddr;
     NetClientState *netdev;
diff --git a/net/net.c b/net/net.c
index 71cccb19da..ab6185b4df 100644
--- a/net/net.c
+++ b/net/net.c
@@ -1012,7 +1012,7 @@ static int net_init_nic(const Netdev *netdev, const char 
*name,
     }
     nd->name = g_strdup(name);
     if (nic->model) {
-        nd->model = g_strdup(nic->model);
+        nd->modelname = g_strdup(nic->model);
     }
     if (nic->addr) {
         nd->devaddr = g_strdup(nic->addr);
@@ -1142,8 +1142,8 @@ NICInfo *qemu_find_nic_info(const char *typename, bool 
match_default,
             continue;
         }
 
-        if ((match_default && !nd->model) || !g_strcmp0(nd->model, typename)
-            || (alias && !g_strcmp0(nd->model, alias))) {
+        if ((match_default && !nd->modelname) || !g_strcmp0(nd->modelname, 
typename)
+            || (alias && !g_strcmp0(nd->modelname, alias))) {
             return nd;
         }
     }
@@ -1210,7 +1210,7 @@ void qemu_create_nic_bus_devices(BusState *bus, const 
char *parent_type,
             continue;
         }
 
-        model = nd->model ? nd->model : default_model;
+        model = nd->modelname ? nd->modelname : default_model;
         if (!model) {
             continue;
         }
@@ -1726,7 +1726,7 @@ void net_check_clients(void)
             warn_report("requested NIC (%s, model %s) "
                         "was not created (not supported by this machine?)",
                         nd->name ? nd->name : "anonymous",
-                        nd->model ? nd->model : "unspecified");
+                        nd->modelname ? nd->modelname : "unspecified");
         }
     }
 }
@@ -1787,9 +1787,9 @@ static int net_param_nic(void *dummy, QemuOpts *opts, 
Error **errp)
 
     ni = &nd_table[idx];
     memset(ni, 0, sizeof(*ni));
-    ni->model = qemu_opt_get_del(opts, "model");
+    ni->modelname = qemu_opt_get_del(opts, "model");
 
-    if (!nic_model_help && !g_strcmp0(ni->model, "help")) {
+    if (!nic_model_help && !g_strcmp0(ni->modelname, "help")) {
         nic_model_help = g_hash_table_new_full(g_str_hash, g_str_equal,
                                                g_free, NULL);
         return 0;

smime.p7s
Description: S/MIME cryptographic signature