Re: [Qemu-devel] [PATCH 14/17] Move daemonize handling to OS specific files

[Frank Arnold]

On Fri, 2010-06-25 at 19:34 +0200, Frank Arnold wrote:
> On Fri, 2010-06-25 at 13:02 -0400, Jes Sorensen wrote:
> > On 06/25/10 18:41, Frank Arnold wrote:
> > > On Thu, 2010-06-10 at 05:42 -0400, address@hidden wrote:
> > >> diff --git a/os-posix.c b/os-posix.c
> > >> index 6417d16..1672e06 100644
> > >> --- a/os-posix.c
> > >> +++ b/os-posix.c
> > >> @@ -160,6 +162,9 @@ void os_parse_cmd_args(int index, const char *optarg)
> > >>      case QEMU_OPTION_chroot:
> > >>          chroot_dir = optarg;
> > >>          break;
> > >> +    case QEMU_OPTION_daemonize:
> > >> +        daemonize = 1;
> > >> +        break;
> > >>      }
> > >>      return;
> > >>  }
> > > 
> > > This move broke the -daemonize option for us. We are using the qemu-kvm
> > > tree.
> > > 
> > > The issue is that the QEMU_OPTION_* enumeration between vl.c and
> > > os-posix.c is out of sync. In our case MAP_POPULATE is defined in vl.c
> > > but is not in os-posix.c. This excludes the option -mem-prealloc in
> > > os-posix.c, see qemu-options.def for the ifdef statement. All subsequent
> > > options are off by one in comparison to vl.c.
> > > 
> > > Just including sys/mman.h in os-posix.c fixes the issue for me. But I'm
> > > not sure if there is a more generic fix to that problem.
> > 
> > Thanks for the update. What do you mean that it changes the numbering,
> > do you get a compile time error or are you saying that it is the order
> > of parsing the options that change?
> > 
> > Are you building on Linux or another OS?
> 
> We are doing KVM testing, so it is Linux.
> 
> What I did is putting lines like this somewhere into vl.c and
> os-posix.c:
> fprintf(stderr, "os: QEMU_OPTION_daemonize: %i", QEMU_OPTION_daemonize);
> fprintf(stderr, "vl: QEMU_OPTION_daemonize: %i", QEMU_OPTION_daemonize);
> 
> Resulting in the following output on stderr:
> os: QEMU_OPTION_daemonize: 85
> vl: QEMU_OPTION_daemonize: 86
> 
> No compile time errors. The preprocessing of qemu-options.h is done
> separately for both files. This results in a missing option definition
> for os-posix.c and discrepancy in the option enumeration.

Sorry, missed the part where your patch comes into play:

>From vl.c the function os_parse_cmd_args is called with option index 86,
and the switch statement in os-posix.c's os_parse_cmd_args checks for 85
to set the daemonize. Obviously, this wont work.

-- Frank