diff mbox series

[v1] Allowing setting and overriding parameters in smb.conf

Message ID 20230801232725.4cc838fb.hc981@poolhem.se
State New
Headers show
Series [v1] Allowing setting and overriding parameters in smb.conf | expand

Commit Message

Henrik Carlqvist Aug. 1, 2023, 9:27 p.m. UTC 
From c480f787981308067a059213a1a7ce9c70ab668e Mon Sep 17 00:00:00 2001
From: Henrik Carlqvist <hc1245@poolhem.se>
Date: Tue, 1 Aug 2023 23:00:15 +0200
Subject: [PATCH] Allowing setting and overriding parameters in smb.conf

Signed-off-by: Henrik Carlqvist <hc1245@poolhem.se>
---

It would be nice to be able to change settings in smb.conf from the qemu 
command line. A kludge to edit the qemu smb.conf of a running smbd process 
is described at https://wiki.archlinux.org/title/QEMU , but IMHO my patch
provides a cleaner solution where parameters can be initially set to the
preferred values.

Best regards Henrik

 net/slirp.c     | 44 ++++++++++++++++++++++++++++++++++++++------
 qapi/net.json   |  3 +++
 qemu-options.hx | 15 ++++++++++++---
 3 files changed, 53 insertions(+), 9 deletions(-)

Comments

Samuel Thibault Aug. 2, 2023, 7:53 p.m. UTC | #1 
Henrik Carlqvist, le mar. 01 août 2023 23:27:25 +0200, a ecrit:
> @@ -950,10 +953,11 @@ static int slirp_smb(SlirpState* s, const char *exported_dir,
>              "printing = bsd\n"
>              "disable spoolss = yes\n"
>              "usershare max shares = 0\n"
> -            "[qemu]\n"
> -            "path=%s\n"
>              "read only=no\n"
>              "guest ok=yes\n"
> +           "%s"
> +            "[qemu]\n"
> +            "path=%s\n"

?This is moving read only and guest ok to the global section?

Samuel
Henrik Carlqvist Aug. 2, 2023, 11:09 p.m. UTC | #2 
On Wed, 2 Aug 2023 21:53:56 +0200
Samuel Thibault <samuel.thibault@gnu.org> wrote:

> Henrik Carlqvist, le mar. 01 août 2023 23:27:25 +0200, a ecrit:
> > @@ -950,10 +953,11 @@ static int slirp_smb(SlirpState* s, const char
> > *exported_dir,
> >              "printing = bsd\n"
> >              "disable spoolss = yes\n"
> >              "usershare max shares = 0\n"
> > -            "[qemu]\n"
> > -            "path=%s\n"
> >              "read only=no\n"
> >              "guest ok=yes\n"
> > +           "%s"
> > +            "[qemu]\n"
> > +            "path=%s\n"
> 
> ?This is moving read only and guest ok to the global section?

Thanks for your quick reply!

Yes, I thought that would be OK because the [qemu] share will inherit those
settings from the global section. By placing those in the global section it is
possible to override them with later contradicting statements like 
"read only=yes\n" in the %s before the [qemu] section.

If I wouldn't have moved them to the global section there could have been a
need to be able to alter parameters both in the global section and the [qemu]
share section as any setting in a share section overrides any global settings.

By only adding custom parameter settings to the end of the global section the
user interface was kept in a more simple way.

regards Henrik
Samuel Thibault Aug. 2, 2023, 11:13 p.m. UTC | #3 
Henrik Carlqvist, le jeu. 03 août 2023 01:09:09 +0200, a ecrit:
> On Wed, 2 Aug 2023 21:53:56 +0200
> Samuel Thibault <samuel.thibault@gnu.org> wrote:
> 
> > Henrik Carlqvist, le mar. 01 août 2023 23:27:25 +0200, a ecrit:
> > > @@ -950,10 +953,11 @@ static int slirp_smb(SlirpState* s, const char
> > > *exported_dir,
> > >              "printing = bsd\n"
> > >              "disable spoolss = yes\n"
> > >              "usershare max shares = 0\n"
> > > -            "[qemu]\n"
> > > -            "path=%s\n"
> > >              "read only=no\n"
> > >              "guest ok=yes\n"
> > > +           "%s"
> > > +            "[qemu]\n"
> > > +            "path=%s\n"
> > 
> > ?This is moving read only and guest ok to the global section?
> 
> Thanks for your quick reply!
> 
> Yes, I thought that would be OK because the [qemu] share will inherit those
> settings from the global section. By placing those in the global section it is
> possible to override them with later contradicting statements like 
> "read only=yes\n" in the %s before the [qemu] section.

Ok, it would be useful to mention that in the changelog.

Thanks,
Samuel
Henrik Carlqvist Aug. 2, 2023, 11:26 p.m. UTC | #4 
On Thu, 3 Aug 2023 01:13:24 +0200
Samuel Thibault <samuel.thibault@gnu.org> wrote:

> Henrik Carlqvist, le jeu. 03 août 2023 01:09:09 +0200, a ecrit:
> > On Wed, 2 Aug 2023 21:53:56 +0200
> > Samuel Thibault <samuel.thibault@gnu.org> wrote:
> > 
> > > Henrik Carlqvist, le mar. 01 août 2023 23:27:25 +0200, a ecrit:
> > > > @@ -950,10 +953,11 @@ static int slirp_smb(SlirpState* s, const char
> > > > *exported_dir,
> > > >              "printing = bsd\n"
> > > >              "disable spoolss = yes\n"
> > > >              "usershare max shares = 0\n"
> > > > -            "[qemu]\n"
> > > > -            "path=%s\n"
> > > >              "read only=no\n"
> > > >              "guest ok=yes\n"
> > > > +           "%s"
> > > > +            "[qemu]\n"
> > > > +            "path=%s\n"
> > > 
> > > ?This is moving read only and guest ok to the global section?
> > 
> > Thanks for your quick reply!
> > 
> > Yes, I thought that would be OK because the [qemu] share will inherit
> > those settings from the global section. By placing those in the global
> > section it is possible to override them with later contradicting
> > statements like "read only=yes\n" in the %s before the [qemu] section.
> 
> Ok, it would be useful to mention that in the changelog.

Is there a changelog in the repo? Or do you mean that I should mention that in
the git commit message?

Best regards Henrik
Samuel Thibault Aug. 2, 2023, 11:34 p.m. UTC | #5 
Henrik Carlqvist, le jeu. 03 août 2023 01:26:02 +0200, a ecrit:
> On Thu, 3 Aug 2023 01:13:24 +0200
> Samuel Thibault <samuel.thibault@gnu.org> wrote:
> 
> > Henrik Carlqvist, le jeu. 03 août 2023 01:09:09 +0200, a ecrit:
> > > On Wed, 2 Aug 2023 21:53:56 +0200
> > > Samuel Thibault <samuel.thibault@gnu.org> wrote:
> > > 
> > > > Henrik Carlqvist, le mar. 01 août 2023 23:27:25 +0200, a ecrit:
> > > > > @@ -950,10 +953,11 @@ static int slirp_smb(SlirpState* s, const char
> > > > > *exported_dir,
> > > > >              "printing = bsd\n"
> > > > >              "disable spoolss = yes\n"
> > > > >              "usershare max shares = 0\n"
> > > > > -            "[qemu]\n"
> > > > > -            "path=%s\n"
> > > > >              "read only=no\n"
> > > > >              "guest ok=yes\n"
> > > > > +           "%s"
> > > > > +            "[qemu]\n"
> > > > > +            "path=%s\n"
> > > > 
> > > > ?This is moving read only and guest ok to the global section?
> > > 
> > > Thanks for your quick reply!
> > > 
> > > Yes, I thought that would be OK because the [qemu] share will inherit
> > > those settings from the global section. By placing those in the global
> > > section it is possible to override them with later contradicting
> > > statements like "read only=yes\n" in the %s before the [qemu] section.
> > 
> > Ok, it would be useful to mention that in the changelog.
> 
> Is there a changelog in the repo? Or do you mean that I should mention that in
> the git commit message?

I mean the latter, yes.

Samuel
Henrik Carlqvist Aug. 3, 2023, 12:13 a.m. UTC | #6 
On Thu, 3 Aug 2023 01:34:04 +0200
Samuel Thibault <samuel.thibault@gnu.org> wrote:

> Henrik Carlqvist, le jeu. 03 août 2023 01:26:02 +0200, a ecrit:
> > On Thu, 3 Aug 2023 01:13:24 +0200
> > Samuel Thibault <samuel.thibault@gnu.org> wrote:
> > 
> > > Henrik Carlqvist, le jeu. 03 août 2023 01:09:09 +0200, a ecrit:
> > > > On Wed, 2 Aug 2023 21:53:56 +0200
> > > > Samuel Thibault <samuel.thibault@gnu.org> wrote:
> > > > 
> > > > > Henrik Carlqvist, le mar. 01 août 2023 23:27:25 +0200, a
> > > > > ecrit:
> > > > > > @@ -950,10 +953,11 @@ static int slirp_smb(SlirpState* s, const
> > > > > > char*exported_dir,
> > > > > >              "printing = bsd\n"
> > > > > >              "disable spoolss = yes\n"
> > > > > >              "usershare max shares = 0\n"
> > > > > > -            "[qemu]\n"
> > > > > > -            "path=%s\n"
> > > > > >              "read only=no\n"
> > > > > >              "guest ok=yes\n"
> > > > > > +           "%s"
> > > > > > +            "[qemu]\n"
> > > > > > +            "path=%s\n"
> > > > > 
> > > > > ?This is moving read only and guest ok to the global section?
> > > > 
> > > > Thanks for your quick reply!
> > > > 
> > > > Yes, I thought that would be OK because the [qemu] share will inherit
> > > > those settings from the global section. By placing those in the global
> > > > section it is possible to override them with later contradicting
> > > > statements like "read only=yes\n" in the %s before the [qemu] section.
> > > 
> > > Ok, it would be useful to mention that in the changelog.
> > 
> > Is there a changelog in the repo? Or do you mean that I should mention
> > that in the git commit message?
> 
> I mean the latter, yes.

Thanks for your feedback! I will provide a new patch tomorrow.

Best regards Henrik
diff mbox series

Patch

diff --git a/net/slirp.c b/net/slirp.c
index c33b3e02e7..f860ea48f6 100644
--- a/net/slirp.c
+++ b/net/slirp.c
@@ -106,7 +106,8 @@  static int slirp_guestfwd(SlirpState *s, const char *config_str, Error **errp);
 
 #if defined(CONFIG_SMBD_COMMAND)
 static int slirp_smb(SlirpState *s, const char *exported_dir,
-                     struct in_addr vserver_addr, Error **errp);
+                     struct in_addr vserver_addr, const char *smbparams,
+                     Error **errp);
 static void slirp_smb_cleanup(SlirpState *s);
 #else
 static inline void slirp_smb_cleanup(SlirpState *s) { }
@@ -424,6 +425,7 @@  static int net_slirp_init(NetClientState *peer, const char *model,
                           const char *bootfile, const char *vdhcp_start,
                           const char *vnameserver, const char *vnameserver6,
                           const char *smb_export, const char *vsmbserver,
+                          const char *smbparams,
                           const char **dnssearch, const char *vdomainname,
                           const char *tftp_server_name,
                           Error **errp)
@@ -678,7 +680,7 @@  static int net_slirp_init(NetClientState *peer, const char *model,
     }
 #if defined(CONFIG_SMBD_COMMAND)
     if (smb_export) {
-        if (slirp_smb(s, smb_export, smbsrv, errp) < 0) {
+        if (slirp_smb(s, smb_export, smbsrv, smbparams, errp) < 0) {
             goto error;
         }
     }
@@ -891,7 +893,8 @@  static void slirp_smb_cleanup(SlirpState *s)
 }
 
 static int slirp_smb(SlirpState* s, const char *exported_dir,
-                     struct in_addr vserver_addr, Error **errp)
+                     struct in_addr vserver_addr, const char *smbparams,
+                     Error **errp)
 {
     char *smb_conf;
     char *smb_cmdline;
@@ -950,10 +953,11 @@  static int slirp_smb(SlirpState* s, const char *exported_dir,
             "printing = bsd\n"
             "disable spoolss = yes\n"
             "usershare max shares = 0\n"
-            "[qemu]\n"
-            "path=%s\n"
             "read only=no\n"
             "guest ok=yes\n"
+           "%s"
+            "[qemu]\n"
+            "path=%s\n"
             "force user=%s\n",
             s->smb_dir,
             s->smb_dir,
@@ -963,6 +967,7 @@  static int slirp_smb(SlirpState* s, const char *exported_dir,
             s->smb_dir,
             s->smb_dir,
             s->smb_dir,
+            smbparams,
             exported_dir,
             passwd->pw_name
             );
@@ -1143,6 +1148,29 @@  static const char **slirp_dnssearch(const StringList *dnsname)
     return ret;
 }
 
+static char *slirp_smbparams(const StringList *smbparam)
+{
+    const StringList *c = smbparam;
+    size_t i = 1; /* for string terminating 0 */
+    char *ret;
+
+    while (c) {
+        i += strlen(c->value->str);
+        i++; /* for \n */
+        c = c->next;
+    }
+    ret = g_malloc(i * sizeof(*ret));
+    ret[0]=0; /* Start with empty string */
+
+    c = smbparam;
+    while (c) {
+        pstrcat(ret, i * sizeof(*ret), c->value->str);
+        pstrcat(ret, i * sizeof(*ret), "\n");
+        c = c->next;
+    }
+    return ret;
+}
+
 int net_init_slirp(const Netdev *netdev, const char *name,
                    NetClientState *peer, Error **errp)
 {
@@ -1151,6 +1179,7 @@  int net_init_slirp(const Netdev *netdev, const char *name,
     int ret;
     const NetdevUserOptions *user;
     const char **dnssearch;
+    char *smbparams;
     bool ipv4 = true, ipv6 = true;
 
     assert(netdev->type == NET_CLIENT_DRIVER_USER);
@@ -1170,6 +1199,7 @@  int net_init_slirp(const Netdev *netdev, const char *name,
            NULL;
 
     dnssearch = slirp_dnssearch(user->dnssearch);
+    smbparams = slirp_smbparams(user->smbparam);
 
     /* all optional fields are initialized to "all bits zero" */
 
@@ -1182,7 +1212,8 @@  int net_init_slirp(const Netdev *netdev, const char *name,
                          user->ipv6_host, user->hostname, user->tftp,
                          user->bootfile, user->dhcpstart,
                          user->dns, user->ipv6_dns, user->smb,
-                         user->smbserver, dnssearch, user->domainname,
+                         user->smbserver, smbparams,
+                         dnssearch, user->domainname,
                          user->tftp_server_name, errp);
 
     while (slirp_configs) {
@@ -1193,6 +1224,7 @@  int net_init_slirp(const Netdev *netdev, const char *name,
 
     g_free(vnet);
     g_free(dnssearch);
+    g_free(smbparams);
 
     return ret;
 }
diff --git a/qapi/net.json b/qapi/net.json
index 313c8a606e..163091719c 100644
--- a/qapi/net.json
+++ b/qapi/net.json
@@ -156,6 +156,8 @@ 
 #
 # @smbserver: IP address of the built-in SMB server
 #
+# @smbparam: list of parameters with values for smb.conf
+#
 # @hostfwd: redirect incoming TCP or UDP host connections to guest
 #     endpoints
 #
@@ -186,6 +188,7 @@ 
     '*ipv6-dns':         'str',
     '*smb':       'str',
     '*smbserver': 'str',
+    '*smbparam': ['String'],
     '*hostfwd':   ['String'],
     '*guestfwd':  ['String'],
     '*tftp-server-name': 'str' } }
diff --git a/qemu-options.hx b/qemu-options.hx
index 29b98c3d4c..7b92d08c3e 100644
--- a/qemu-options.hx
+++ b/qemu-options.hx
@@ -2758,9 +2758,9 @@  DEF("netdev", HAS_ARG, QEMU_OPTION_netdev,
     "         [,ipv6=on|off][,ipv6-net=addr[/int]][,ipv6-host=addr]\n"
     "         [,restrict=on|off][,hostname=host][,dhcpstart=addr]\n"
     "         [,dns=addr][,ipv6-dns=addr][,dnssearch=domain][,domainname=domain]\n"
-    "         [,tftp=dir][,tftp-server-name=name][,bootfile=f][,hostfwd=rule][,guestfwd=rule]"
+    "         [,tftp=dir][,tftp-server-name=name][,bootfile=f][,hostfwd=rule][,guestfwd=rule]\n"
 #ifndef _WIN32
-                                             "[,smb=dir[,smbserver=addr]]\n"
+    "         [,smb=dir[,smbserver=addr][,smbparam=parameter=value]]\n"
 #endif
     "                configure a user mode network backend with ID 'str',\n"
     "                its DHCP server and optional services\n"
@@ -3062,7 +3062,7 @@  SRST
             |qemu_system| -hda linux.img -boot n -device e1000,netdev=n1 \\
                 -netdev user,id=n1,tftp=/path/to/tftp/files,bootfile=/pxelinux.0
 
-    ``smb=dir[,smbserver=addr]``
+    ``smb=dir[,smbserver=addr][,smbparam=parameter=value]``
         When using the user mode network stack, activate a built-in SMB
         server so that Windows OSes can access to the host files in
         ``dir`` transparently. The IP address of the SMB server can be
@@ -3081,6 +3081,15 @@  SRST
 
         Then ``dir`` can be accessed in ``\\smbserver\qemu``.
 
+        It is possible to set samba parameters in the generated smb.conf
+        with one or more ``smbparam=parameter=value``. Example:
+
+        .. parsed-literal::
+
+            |qemu_system| -nic user,smb=/tmp,smbparam="read only"=yes,smbparam="server min protocol"=NT1
+
+        See the man page of smb.conf for a complete listing of parameters.
+
         Note that a SAMBA server must be installed on the host OS.
 
     ``hostfwd=[tcp|udp]:[hostaddr]:hostport-[guestaddr]:guestport``