Re: [Qemu-devel] [PATCH 00/16] clean up vl.c code
On 06/09/10 09:07, Markus Armbruster wrote: > Jes Sorensen writes: >> On 06/04/10 13:54, Markus Armbruster wrote: >> If there is strong feeling we should do it this way instead, I can >> change the code to do it this way instead. I am not married to the >> current approach, I just find it the lesser evil. > > I'm making suggestions, not demands. If you still prefer your way after > considering my suggestions, go with it. > > For what it's worth, pushing OS-dependent bits out of the option parsing > allows for: > > qemu: --frobnicate: Can't frobnicate under Windows > > instead of > > qemu: --frobnicate: invalid option That is true, on the other hand I think it would be cleaner to not advertise options on an OS where it is not supported. Ideally I would like them to not show up in qemu --help at all. Cheers, Jes
Re: [Qemu-devel] [PATCH 00/16] clean up vl.c code
Jes Sorensen writes: > On 06/04/10 13:54, Markus Armbruster wrote: >> Jes Sorensen writes: >> >>> On 06/04/10 10:21, Markus Armbruster wrote: I like moving stuff out of vl.c in general. Your moves of entire functions look like a win to me. I have doubts about spreading the option switch over three files, though. >>> >>> The problem is right now there are too many OS specific options, but >>> having the #ifdefs plastered all over to enable/disable them accordingly >>> is just a nightmare and is prone to leave in inconsistent behavior for >>> various OSes. See the set_proc_name() stuff for an example. >> >> I doubt spreading option code over separate files will help consistency. >> >> I suspect the true root of the problem is having (too many) OS-specific >> options in the first place. What about parsing options the same >> everywhere, calling out to OS-specific functions to do the actual work? >> Let them fail with "can't do this on this OS". > > That is a possibility which I did consider, but it would end up in far > more os specific functions for simple assignments etc. Far more? option condition what's needed to make it uncond. QEMU_OPTION_smb !_WIN32 net_slirp_smb() dummy. QEMU_OPTION_fsdev CONFIG_LINUXfsdev_init_func() dummy QEMU_OPTION_virtfs CONFIG_LINUXnothing, it's just short for -fsdev & -device QEMU_OPTION_daemonize !_WIN32 move the big #ifndef _WIN32 chunk at the end of main() into a helper, + dummy QEMU_OPTION_chroot !_WIN32 ditto QEMU_OPTION_runas !_WIN32 ditto >I modeled it the > way I did similar to how we handle ioctl calls in the kernel. > > If there is strong feeling we should do it this way instead, I can > change the code to do it this way instead. I am not married to the > current approach, I just find it the lesser evil. I'm making suggestions, not demands. If you still prefer your way after considering my suggestions, go with it. For what it's worth, pushing OS-dependent bits out of the option parsing allows for: qemu: --frobnicate: Can't frobnicate under Windows instead of qemu: --frobnicate: invalid option
Re: [Qemu-devel] [PATCH 00/16] clean up vl.c code
On 06/04/10 13:54, Markus Armbruster wrote: > Jes Sorensen writes: > >> On 06/04/10 10:21, Markus Armbruster wrote: >>> I like moving stuff out of vl.c in general. Your moves of entire >>> functions look like a win to me. I have doubts about spreading the >>> option switch over three files, though. >> >> The problem is right now there are too many OS specific options, but >> having the #ifdefs plastered all over to enable/disable them accordingly >> is just a nightmare and is prone to leave in inconsistent behavior for >> various OSes. See the set_proc_name() stuff for an example. > > I doubt spreading option code over separate files will help consistency. > > I suspect the true root of the problem is having (too many) OS-specific > options in the first place. What about parsing options the same > everywhere, calling out to OS-specific functions to do the actual work? > Let them fail with "can't do this on this OS". That is a possibility which I did consider, but it would end up in far more os specific functions for simple assignments etc. I modeled it the way I did similar to how we handle ioctl calls in the kernel. If there is strong feeling we should do it this way instead, I can change the code to do it this way instead. I am not married to the current approach, I just find it the lesser evil. Cheers, Jes
Re: [Qemu-devel] [PATCH 00/16] clean up vl.c code
Jes Sorensen writes: > On 06/04/10 10:21, Markus Armbruster wrote: >> jes.soren...@redhat.com writes: >>> I have tried to be as careful as I can to not break non Linux support, >>> but as I only have a Linux build environment handy, I would appreciate >>> it if people with other OSes could check that I didn't break anything >>> for them. In particular I would like to know if win32 still builds. >> >> I like moving stuff out of vl.c in general. Your moves of entire >> functions look like a win to me. I have doubts about spreading the >> option switch over three files, though. > > The problem is right now there are too many OS specific options, but > having the #ifdefs plastered all over to enable/disable them accordingly > is just a nightmare and is prone to leave in inconsistent behavior for > various OSes. See the set_proc_name() stuff for an example. I doubt spreading option code over separate files will help consistency. I suspect the true root of the problem is having (too many) OS-specific options in the first place. What about parsing options the same everywhere, calling out to OS-specific functions to do the actual work? Let them fail with "can't do this on this OS".
Re: [Qemu-devel] [PATCH 00/16] clean up vl.c code
On 06/04/10 10:21, Markus Armbruster wrote: > jes.soren...@redhat.com writes: >> I have tried to be as careful as I can to not break non Linux support, >> but as I only have a Linux build environment handy, I would appreciate >> it if people with other OSes could check that I didn't break anything >> for them. In particular I would like to know if win32 still builds. > > I like moving stuff out of vl.c in general. Your moves of entire > functions look like a win to me. I have doubts about spreading the > option switch over three files, though. The problem is right now there are too many OS specific options, but having the #ifdefs plastered all over to enable/disable them accordingly is just a nightmare and is prone to leave in inconsistent behavior for various OSes. See the set_proc_name() stuff for an example. Cheers, Jes
Re: [Qemu-devel] [PATCH 00/16] clean up vl.c code
jes.soren...@redhat.com writes: > From: Jes Sorensen > > Hi, > > I have been working on a set of patches to clean up the vl.c code, by > separating out OS specific code into OS specific files. Basically it > introduces two header files: qemu-os-win32.h and qemu-os-posix.h as > well as os-win32.c and os-posix.c. > > I have tried to be as careful as I can to not break non Linux support, > but as I only have a Linux build environment handy, I would appreciate > it if people with other OSes could check that I didn't break anything > for them. In particular I would like to know if win32 still builds. I like moving stuff out of vl.c in general. Your moves of entire functions look like a win to me. I have doubts about spreading the option switch over three files, though.
[Qemu-devel] [PATCH 00/16] clean up vl.c code
From: Jes Sorensen Hi, I have been working on a set of patches to clean up the vl.c code, by separating out OS specific code into OS specific files. Basically it introduces two header files: qemu-os-win32.h and qemu-os-posix.h as well as os-win32.c and os-posix.c. I have tried to be as careful as I can to not break non Linux support, but as I only have a Linux build environment handy, I would appreciate it if people with other OSes could check that I didn't break anything for them. In particular I would like to know if win32 still builds. Thanks, Jes Jes Sorensen (16): vl.c: Remove double include of netinet/in.h for Solaris Create qemu-os-win32.h and move WIN32 specific declarations there Introduce os-win32.c and move polling functions from vl.c vl.c: Move host_main_loop_wait() to OS specific files. Introduce os-posix.c and create os_setup_signal_handling() Move win32 early signal handling setup to os_setup_signal_handling() Rename os_setup_signal_handling() to os_setup_early_signal_handling() Move main signal handler setup to os specificfiles. Move find_datadir to OS specific files. Introduce OS specific cmdline argument handling and move SMB arg to os-posix.c Move runas handling from vl.c to OS specific files. Move chroot handling to OS specific files. Move daemonize handling to OS specific files Make os_change_process_uid and os_change_root os-posix.c local Move line-buffering setup to OS specific files. Move set_proc_name() to OS specific files. Makefile.objs |2 + os-posix.c | 344 ++ os-win32.c | 233 ++ qemu-os-posix.h | 39 + qemu-os-win32.h | 52 ++ sysemu.h| 35 ++-- vl.c| 490 ++- 7 files changed, 703 insertions(+), 492 deletions(-) create mode 100644 os-posix.c create mode 100644 os-win32.c create mode 100644 qemu-os-posix.h create mode 100644 qemu-os-win32.h
