Re: [Qemu-devel] [PATCH] slirp: Ensure smbd and shared directory exist when enable smb

[Jan Kiszka]

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 <address@hidden>
> ---
>  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

signature.asc
Description: OpenPGP digital signature