slirp: Ensure smbd and shared directory exist when enable smb

Submitted by dunrong huang on July 3, 2012, 1:13 p.m.

Details

Message ID 1341321192-30258-1-git-send-email-riegamaths@gmail.com
State New
Headers show

Commit Message

dunrong huang July 3, 2012, 1:13 p.m.
Users may pass the following parameters to qemu:
    $ qemu-kvm -net nic -net user,smb= ...
    $ qemu-kvm -net nic -net user,smb ...
    $ qemu-kvm -net nic -net user,smb=bad_directory ...

In these cases, qemu started successfully while samba server
failed to start. Users will confuse since samba server
failed silently without any indication of what it did wrong.

To avoid it, we check whether the shared directory exists or
not when QEMU's "built-in" SMB server is enabled.

Signed-off-by: Dunrong Huang <riegamaths@gmail.com>
---
 net/slirp.c |   14 ++++++++++++++
 1 files changed, 14 insertions(+), 0 deletions(-)

Comments

Jan Kiszka July 5, 2012, 11:18 p.m.
Sorry for the late review. Two comments below.

On 2012-07-03 15:13, Dunrong Huang wrote:
> Users may pass the following parameters to qemu:
>     $ qemu-kvm -net nic -net user,smb= ...
>     $ qemu-kvm -net nic -net user,smb ...
>     $ qemu-kvm -net nic -net user,smb=bad_directory ...
> 
> In these cases, qemu started successfully while samba server
> failed to start. Users will confuse since samba server
> failed silently without any indication of what it did wrong.
> 
> To avoid it, we check whether the shared directory exists or
> not when QEMU's "built-in" SMB server is enabled.
> 
> Signed-off-by: Dunrong Huang <riegamaths@gmail.com>
> ---
>  net/slirp.c |   14 ++++++++++++++
>  1 files changed, 14 insertions(+), 0 deletions(-)
> 
> diff --git a/net/slirp.c b/net/slirp.c
> index 37b6ccf..a672cff 100644
> --- a/net/slirp.c
> +++ b/net/slirp.c
> @@ -489,6 +489,20 @@ static int slirp_smb(SlirpState* s, const char *exported_dir,
>      char smb_cmdline[128];
>      FILE *f;
>  
> +    if (access(CONFIG_SMBD_COMMAND, F_OK)) {
> +        slirp_smb_cleanup(s);

Both slirp_smb_cleanup are redundant at this point, please remove them.

> +        error_report("could not find '%s', please install it",
> +                     CONFIG_SMBD_COMMAND);
> +        return -1;
> +    }
> +
> +    if (access(exported_dir, F_OK)) {

What about checking for R_OK | X_OK to avoid that we run into lacking
permissions later on?

> +        slirp_smb_cleanup(s);
> +        error_report("no such shared directory '%s', please check it",
> +                     exported_dir);
> +        return -1;
> +    }
> +
>      snprintf(s->smb_dir, sizeof(s->smb_dir), "/tmp/qemu-smb.%ld-%d",
>               (long)getpid(), instance++);
>      if (mkdir(s->smb_dir, 0700) < 0) {
> 

Jan
dunrong huang July 6, 2012, 5:55 a.m.
Thanks for your reply.

2012/7/6 Jan Kiszka <jan.kiszka@web.de>:

>> diff --git a/net/slirp.c b/net/slirp.c
>> index 37b6ccf..a672cff 100644
>> --- a/net/slirp.c
>> +++ b/net/slirp.c
>> @@ -489,6 +489,20 @@ static int slirp_smb(SlirpState* s, const char *exported_dir,
>>      char smb_cmdline[128];
>>      FILE *f;
>>
>> +    if (access(CONFIG_SMBD_COMMAND, F_OK)) {
>> +        slirp_smb_cleanup(s);
>
> Both slirp_smb_cleanup are redundant at this point, please remove them.
You are right, i will fix it
>
>> +        error_report("could not find '%s', please install it",
>> +                     CONFIG_SMBD_COMMAND);
>> +        return -1;
>> +    }
>> +
>> +    if (access(exported_dir, F_OK)) {
>
> What about checking for R_OK | X_OK to avoid that we run into lacking
> permissions later on?
>
I agree with you, I will send a v2 patch

Patch hide | download patch | download mbox

diff --git a/net/slirp.c b/net/slirp.c
index 37b6ccf..a672cff 100644
--- a/net/slirp.c
+++ b/net/slirp.c
@@ -489,6 +489,20 @@  static int slirp_smb(SlirpState* s, const char *exported_dir,
     char smb_cmdline[128];
     FILE *f;
 
+    if (access(CONFIG_SMBD_COMMAND, F_OK)) {
+        slirp_smb_cleanup(s);
+        error_report("could not find '%s', please install it",
+                     CONFIG_SMBD_COMMAND);
+        return -1;
+    }
+
+    if (access(exported_dir, F_OK)) {
+        slirp_smb_cleanup(s);
+        error_report("no such shared directory '%s', please check it",
+                     exported_dir);
+        return -1;
+    }
+
     snprintf(s->smb_dir, sizeof(s->smb_dir), "/tmp/qemu-smb.%ld-%d",
              (long)getpid(), instance++);
     if (mkdir(s->smb_dir, 0700) < 0) {