From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([208.118.235.92]:42355) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1SmvJM-0004qi-3E for qemu-devel@nongnu.org; Thu, 05 Jul 2012 19:18:13 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1SmvJK-0004OI-6i for qemu-devel@nongnu.org; Thu, 05 Jul 2012 19:18:11 -0400 Received: from mout.web.de ([212.227.15.4]:62496) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1SmvJJ-0004O9-TQ for qemu-devel@nongnu.org; Thu, 05 Jul 2012 19:18:10 -0400 Message-ID: <4FF620AA.2050706@web.de> Date: Fri, 06 Jul 2012 01:18:02 +0200 From: Jan Kiszka MIME-Version: 1.0 References: <1341321192-30258-1-git-send-email-riegamaths@gmail.com> In-Reply-To: <1341321192-30258-1-git-send-email-riegamaths@gmail.com> Content-Type: multipart/signed; micalg=pgp-sha1; protocol="application/pgp-signature"; boundary="------------enigF5680BB50644529683792B0E" Subject: Re: [Qemu-devel] [PATCH] slirp: Ensure smbd and shared directory exist when enable smb List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Dunrong Huang Cc: ark McLoughlin , Anthony Liguori , qemu-devel@nongnu.org This is an OpenPGP/MIME signed message (RFC 2440 and 3156) --------------enigF5680BB50644529683792B0E Content-Type: text/plain; charset=ISO-8859-15 Content-Transfer-Encoding: quoted-printable 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=3D ... > $ qemu-kvm -net nic -net user,smb ... > $ qemu-kvm -net nic -net user,smb=3Dbad_directory ... >=20 > 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. >=20 > To avoid it, we check whether the shared directory exists or > not when QEMU's "built-in" SMB server is enabled. >=20 > Signed-off-by: Dunrong Huang > --- > net/slirp.c | 14 ++++++++++++++ > 1 files changed, 14 insertions(+), 0 deletions(-) >=20 > 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 *ex= ported_dir, > char smb_cmdline[128]; > FILE *f; > =20 > + 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) { >=20 Jan --------------enigF5680BB50644529683792B0E Content-Type: application/pgp-signature; name="signature.asc" Content-Description: OpenPGP digital signature Content-Disposition: attachment; filename="signature.asc" -----BEGIN PGP SIGNATURE----- Version: GnuPG v2.0.16 (GNU/Linux) Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org/ iEYEARECAAYFAk/2IKoACgkQitSsb3rl5xQuOQCfdFHC5NovT42w+IolC/+NdwpN jHYAoLWYTRHnTL0+vaHU2XZ2zemfFyxs =H1+i -----END PGP SIGNATURE----- --------------enigF5680BB50644529683792B0E--