diff mbox

do not chdir(/) in qemu-nbd

Message ID 20120113090951.3D333341@gandalf.tls.msk.ru
State New
Headers show

Commit Message

Michael Tokarev Jan. 13, 2012, 9:04 a.m. UTC
When qemu-nbd becomes a daemon it calls daemon(3) with
nochdir=0, so daemon(3) changes current directory to /.
But at this time, qemu-nbd did not open any user-specified
files yet, so by changing current directory, all non-absolute
paths becomes wrong.  The solution is to pass nochdir=1 to
daemon(3) function.

This patch is applicable for -stable.

Signed-Off-By: Michael Tokarev <mjt@tls.msk.ru>
---
 qemu-nbd.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

Comments

Stefan Hajnoczi Jan. 13, 2012, 11:57 a.m. UTC | #1
On Fri, Jan 13, 2012 at 9:04 AM, Michael Tokarev <mjt@tls.msk.ru> wrote:
> When qemu-nbd becomes a daemon it calls daemon(3) with
> nochdir=0, so daemon(3) changes current directory to /.
> But at this time, qemu-nbd did not open any user-specified
> files yet, so by changing current directory, all non-absolute
> paths becomes wrong.  The solution is to pass nochdir=1 to
> daemon(3) function.

It's polite to chdir("/") so that file systems can be unmounted (even
more important when chroot was involved, but I think qemu-nbd doesn't
do that).  Is it possible to manually do a chdir("/") later on after
we've opened necessary files?

Stefan
Michael Tokarev Jan. 13, 2012, 12:47 p.m. UTC | #2
On 13.01.2012 15:57, Stefan Hajnoczi wrote:
> On Fri, Jan 13, 2012 at 9:04 AM, Michael Tokarev <mjt@tls.msk.ru> wrote:
>> When qemu-nbd becomes a daemon it calls daemon(3) with
>> nochdir=0, so daemon(3) changes current directory to /.
>> But at this time, qemu-nbd did not open any user-specified
>> files yet, so by changing current directory, all non-absolute
>> paths becomes wrong.  The solution is to pass nochdir=1 to
>> daemon(3) function.
> 
> It's polite to chdir("/") so that file systems can be unmounted (even
> more important when chroot was involved, but I think qemu-nbd doesn't
> do that).  Is it possible to manually do a chdir("/") later on after
> we've opened necessary files?

Yes that was something I wasn't happy about too -- lack of chdir(/) in
daemons is annoying.

But instead of adding a chdir later, I'll try to rearrange code a bit
to do all init in the parent instead.

Will send a follow-up.

Thanks,

/mjt
Daniel P. Berrangé Jan. 13, 2012, 1:01 p.m. UTC | #3
On Fri, Jan 13, 2012 at 04:47:35PM +0400, Michael Tokarev wrote:
> On 13.01.2012 15:57, Stefan Hajnoczi wrote:
> > On Fri, Jan 13, 2012 at 9:04 AM, Michael Tokarev <mjt@tls.msk.ru> wrote:
> >> When qemu-nbd becomes a daemon it calls daemon(3) with
> >> nochdir=0, so daemon(3) changes current directory to /.
> >> But at this time, qemu-nbd did not open any user-specified
> >> files yet, so by changing current directory, all non-absolute
> >> paths becomes wrong.  The solution is to pass nochdir=1 to
> >> daemon(3) function.
> > 
> > It's polite to chdir("/") so that file systems can be unmounted (even
> > more important when chroot was involved, but I think qemu-nbd doesn't
> > do that).  Is it possible to manually do a chdir("/") later on after
> > we've opened necessary files?
> 
> Yes that was something I wasn't happy about too -- lack of chdir(/) in
> daemons is annoying.
> 
> But instead of adding a chdir later, I'll try to rearrange code a bit
> to do all init in the parent instead.

Or just canonicalize all relative paths before daemonizing.


Daniel
Paolo Bonzini Jan. 13, 2012, 1:41 p.m. UTC | #4
On 01/13/2012 01:47 PM, Michael Tokarev wrote:
> But instead of adding a chdir later, I'll try to rearrange code a bit
> to do all init in the parent instead.

That's not possible, because when you fork you lose all threads except 
the main thread.  That's why the daemon() was moved very early.  Your 
patch is okay if you also add a chdir("/") later on.

Paolo
diff mbox

Patch

diff --git a/qemu-nbd.c b/qemu-nbd.c
index eb61c33..d84e2a7 100644
--- a/qemu-nbd.c
+++ b/qemu-nbd.c
@@ -429,7 +429,7 @@  int main(int argc, char **argv)
         pid = fork();
         if (pid == 0) {
             close(stderr_fd[0]);
-            ret = qemu_daemon(0, 0);
+            ret = qemu_daemon(1, 0);
 
             /* Temporarily redirect stderr to the parent's pipe...  */
             dup2(stderr_fd[1], STDERR_FILENO);