[Qemu-devel] [PATCH] do not chdir(/) in qemu-nbd

[Qemu-devel] [PATCH] do not chdir(/) in qemu-nbd

[Michael Tokarev]

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(-)

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);
-- 
1.7.2.5

Re: [Qemu-devel] [PATCH] do not chdir(/) in qemu-nbd

[Stefan Hajnoczi]

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

Re: [Qemu-devel] [PATCH] do not chdir(/) in qemu-nbd

[Michael Tokarev]

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

Re: [Qemu-devel] [PATCH] do not chdir(/) in qemu-nbd

[Daniel P. Berrange]

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
-- 
|: http://berrange.com      -o-    http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org              -o-             http://virt-manager.org :|
|: http://autobuild.org       -o-         http://search.cpan.org/~danberr/ :|
|: http://entangle-photo.org       -o-       http://live.gnome.org/gtk-vnc :|

Re: [Qemu-devel] [PATCH] do not chdir(/) in qemu-nbd

[Paolo Bonzini]

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