| Message ID | 1425466773-8532-1-git-send-email-zhang.zhanghailiang@huawei.com |
|---|---|
| State | New |
| Headers | show |
On 04/03/2015 11:59, zhanghailiang wrote: > From: Xiangyou Xie <xiexiangyou@huawei.com> > > If VM is configured with large size of hugepage, when startup, > it will consume lots of time to zero the hugepage memory in the function > 'os_mem_prealloc'. > Libvirtd will wait 30 seconds for qemu to establish the monitor, > If the timeout triggers, libvirtd will send TERM signal to kill qemu. > > To solve the problem, adjust the processing of '-mon' to the ahead of '-object'. > > Signed-off-by: Xiangyou Xie <xiexiangyou@huawei.com> > Signed-off-by: zhanghailiang <zhang.zhanghailiang@huawei.com> > --- > vl.c | 8 ++++---- > 1 file changed, 4 insertions(+), 4 deletions(-) > > diff --git a/vl.c b/vl.c > index 86bdce0..d0c03fe 100644 > --- a/vl.c > +++ b/vl.c > @@ -4000,6 +4000,10 @@ int main(int argc, char **argv, char **envp) > exit(0); > } > > + if (qemu_opts_foreach(qemu_find_opts("mon"), mon_init_func, NULL, 1) != 0) { > + exit(1); > + } > + > if (qemu_opts_foreach(qemu_find_opts("object"), > object_create, NULL, 0) != 0) { > exit(1); > @@ -4154,10 +4158,6 @@ int main(int argc, char **argv, char **envp) > > parse_numa_opts(); > > - if (qemu_opts_foreach(qemu_find_opts("mon"), mon_init_func, NULL, 1) != 0) { > - exit(1); > - } > - > if (foreach_device_config(DEV_SERIAL, serial_parse) < 0) > exit(1); > if (foreach_device_config(DEV_PARALLEL, parallel_parse) < 0) > Reviewed-by: Paolo Bonzini <pbonzini@redhat.com>
zhanghailiang <zhang.zhanghailiang@huawei.com> writes: > From: Xiangyou Xie <xiexiangyou@huawei.com> > > If VM is configured with large size of hugepage, when startup, > it will consume lots of time to zero the hugepage memory in the function > 'os_mem_prealloc'. > Libvirtd will wait 30 seconds for qemu to establish the monitor, > If the timeout triggers, libvirtd will send TERM signal to kill qemu. > > To solve the problem, adjust the processing of '-mon' to the ahead of '-object'. > > Signed-off-by: Xiangyou Xie <xiexiangyou@huawei.com> > Signed-off-by: zhanghailiang <zhang.zhanghailiang@huawei.com> > --- > vl.c | 8 ++++---- > 1 file changed, 4 insertions(+), 4 deletions(-) > > diff --git a/vl.c b/vl.c > index 86bdce0..d0c03fe 100644 > --- a/vl.c > +++ b/vl.c > @@ -4000,6 +4000,10 @@ int main(int argc, char **argv, char **envp) > exit(0); > } > > + if (qemu_opts_foreach(qemu_find_opts("mon"), mon_init_func, NULL, 1) != 0) { > + exit(1); > + } > + > if (qemu_opts_foreach(qemu_find_opts("object"), > object_create, NULL, 0) != 0) { > exit(1); > @@ -4154,10 +4158,6 @@ int main(int argc, char **argv, char **envp) > > parse_numa_opts(); > > - if (qemu_opts_foreach(qemu_find_opts("mon"), mon_init_func, NULL, 1) != 0) { > - exit(1); > - } > - > if (foreach_device_config(DEV_SERIAL, serial_parse) < 0) > exit(1); > if (foreach_device_config(DEV_PARALLEL, parallel_parse) < 0) Errors after monitor initialization look ugly when a monitor is on stdio: $ qemu -nodefaults -monitor stdio -vga xxx QEMU 2.2.50 monitor - type 'help' for more information (qemu) Unknown vga type: xxx $ This patch initializes monitors earlier, thus makes more errors look ugly. Do we care? If not: should we initialize them even earlier? They depend on character devices, so what about right after loop calling chardev_init_func()? Our startup is a big happy ad hoc mess. A more organizes program would read and check configuration first (quick, can fail), then allocate resources (quick, can fail), then initialize (somewhat slow, failure unlikely).
On 2015/3/5 1:02, Markus Armbruster wrote: > zhanghailiang <zhang.zhanghailiang@huawei.com> writes: > >> From: Xiangyou Xie <xiexiangyou@huawei.com> >> >> If VM is configured with large size of hugepage, when startup, >> it will consume lots of time to zero the hugepage memory in the function >> 'os_mem_prealloc'. >> Libvirtd will wait 30 seconds for qemu to establish the monitor, >> If the timeout triggers, libvirtd will send TERM signal to kill qemu. >> >> To solve the problem, adjust the processing of '-mon' to the ahead of '-object'. >> >> Signed-off-by: Xiangyou Xie <xiexiangyou@huawei.com> >> Signed-off-by: zhanghailiang <zhang.zhanghailiang@huawei.com> >> --- >> vl.c | 8 ++++---- >> 1 file changed, 4 insertions(+), 4 deletions(-) >> >> diff --git a/vl.c b/vl.c >> index 86bdce0..d0c03fe 100644 >> --- a/vl.c >> +++ b/vl.c >> @@ -4000,6 +4000,10 @@ int main(int argc, char **argv, char **envp) >> exit(0); >> } >> >> + if (qemu_opts_foreach(qemu_find_opts("mon"), mon_init_func, NULL, 1) != 0) { >> + exit(1); >> + } >> + >> if (qemu_opts_foreach(qemu_find_opts("object"), >> object_create, NULL, 0) != 0) { >> exit(1); >> @@ -4154,10 +4158,6 @@ int main(int argc, char **argv, char **envp) >> >> parse_numa_opts(); >> >> - if (qemu_opts_foreach(qemu_find_opts("mon"), mon_init_func, NULL, 1) != 0) { >> - exit(1); >> - } >> - >> if (foreach_device_config(DEV_SERIAL, serial_parse) < 0) >> exit(1); >> if (foreach_device_config(DEV_PARALLEL, parallel_parse) < 0) > > > Errors after monitor initialization look ugly when a monitor is on > stdio: > > $ qemu -nodefaults -monitor stdio -vga xxx > QEMU 2.2.50 monitor - type 'help' for more information > (qemu) Unknown vga type: xxx > $ > > This patch initializes monitors earlier, thus makes more errors look > ugly. Do we care? > Hmm, i don't think it will make any difference, no matter monitor or stdio, it is all OK, it is just error reports, let people know what's wrong. > If not: should we initialize them even earlier? They depend on > character devices, so what about right after loop calling > chardev_init_func()? > This is reasonable, will fix it like that in v2, thanks. > Our startup is a big happy ad hoc mess. A more organizes program would Yes, maybe some cleanup work should be done. > read and check configuration first (quick, can fail), then allocate > resources (quick, can fail), then initialize (somewhat slow, failure > unlikely). > > . >
On 5 March 2015 at 02:02, Markus Armbruster <armbru@redhat.com> wrote: > zhanghailiang <zhang.zhanghailiang@huawei.com> writes: > >> From: Xiangyou Xie <xiexiangyou@huawei.com> >> >> If VM is configured with large size of hugepage, when startup, >> it will consume lots of time to zero the hugepage memory in the function >> 'os_mem_prealloc'. >> Libvirtd will wait 30 seconds for qemu to establish the monitor, >> If the timeout triggers, libvirtd will send TERM signal to kill qemu. >> >> To solve the problem, adjust the processing of '-mon' to the ahead of '-object'. >> >> Signed-off-by: Xiangyou Xie <xiexiangyou@huawei.com> >> Signed-off-by: zhanghailiang <zhang.zhanghailiang@huawei.com> >> --- >> vl.c | 8 ++++---- >> 1 file changed, 4 insertions(+), 4 deletions(-) >> >> diff --git a/vl.c b/vl.c >> index 86bdce0..d0c03fe 100644 >> --- a/vl.c >> +++ b/vl.c >> @@ -4000,6 +4000,10 @@ int main(int argc, char **argv, char **envp) >> exit(0); >> } >> >> + if (qemu_opts_foreach(qemu_find_opts("mon"), mon_init_func, NULL, 1) != 0) { >> + exit(1); >> + } >> + >> if (qemu_opts_foreach(qemu_find_opts("object"), >> object_create, NULL, 0) != 0) { >> exit(1); >> @@ -4154,10 +4158,6 @@ int main(int argc, char **argv, char **envp) >> >> parse_numa_opts(); >> >> - if (qemu_opts_foreach(qemu_find_opts("mon"), mon_init_func, NULL, 1) != 0) { >> - exit(1); >> - } >> - >> if (foreach_device_config(DEV_SERIAL, serial_parse) < 0) >> exit(1); >> if (foreach_device_config(DEV_PARALLEL, parallel_parse) < 0) > > > Errors after monitor initialization look ugly when a monitor is on > stdio: > > $ qemu -nodefaults -monitor stdio -vga xxx > QEMU 2.2.50 monitor - type 'help' for more information > (qemu) Unknown vga type: xxx > $ > > This patch initializes monitors earlier, thus makes more errors look > ugly. Do we care? Yeah, this doesn't seem great. Surely the actual problem here is that we're doing something that takes 30 seconds to initialise as part of our command line option parsing phase ? -- PMM
On 2015/3/6 17:51, Peter Maydell wrote: > On 5 March 2015 at 02:02, Markus Armbruster <armbru@redhat.com> wrote: >> zhanghailiang <zhang.zhanghailiang@huawei.com> writes: >> >>> From: Xiangyou Xie <xiexiangyou@huawei.com> >>> >>> If VM is configured with large size of hugepage, when startup, >>> it will consume lots of time to zero the hugepage memory in the function >>> 'os_mem_prealloc'. >>> Libvirtd will wait 30 seconds for qemu to establish the monitor, >>> If the timeout triggers, libvirtd will send TERM signal to kill qemu. >>> >>> To solve the problem, adjust the processing of '-mon' to the ahead of '-object'. >>> >>> Signed-off-by: Xiangyou Xie <xiexiangyou@huawei.com> >>> Signed-off-by: zhanghailiang <zhang.zhanghailiang@huawei.com> >>> --- >>> vl.c | 8 ++++---- >>> 1 file changed, 4 insertions(+), 4 deletions(-) >>> >>> diff --git a/vl.c b/vl.c >>> index 86bdce0..d0c03fe 100644 >>> --- a/vl.c >>> +++ b/vl.c >>> @@ -4000,6 +4000,10 @@ int main(int argc, char **argv, char **envp) >>> exit(0); >>> } >>> >>> + if (qemu_opts_foreach(qemu_find_opts("mon"), mon_init_func, NULL, 1) != 0) { >>> + exit(1); >>> + } >>> + >>> if (qemu_opts_foreach(qemu_find_opts("object"), >>> object_create, NULL, 0) != 0) { >>> exit(1); >>> @@ -4154,10 +4158,6 @@ int main(int argc, char **argv, char **envp) >>> >>> parse_numa_opts(); >>> >>> - if (qemu_opts_foreach(qemu_find_opts("mon"), mon_init_func, NULL, 1) != 0) { >>> - exit(1); >>> - } >>> - >>> if (foreach_device_config(DEV_SERIAL, serial_parse) < 0) >>> exit(1); >>> if (foreach_device_config(DEV_PARALLEL, parallel_parse) < 0) >> >> >> Errors after monitor initialization look n a monitor is on >> stdio: >> >> $ qemu -nodefaults -monitor stdio -vga xxx >> QEMU 2.2.50 monitor - type 'help' for more information >> (qemu) Unknown vga type: xxx >> $ >> >> This patch initializes monitors earlier, thus makes more errors look >> ugly. Do we care? > > Yeah, this doesn't seem great. Surely the actual problem here > is that we're doing something that takes 30 seconds to initialise > as part of our command line option parsing phase ? > Yes, it is the problem of memset() in os_mem_prealloc, if we configure VM with large size memory (prealloc is also configured), it will consume lots of time. we can certainly do this consuming time work later, after command line option parsing phase, but this scenario is more easy. > -- PMM > > . >
Peter Maydell <peter.maydell@linaro.org> writes: > On 5 March 2015 at 02:02, Markus Armbruster <armbru@redhat.com> wrote: >> zhanghailiang <zhang.zhanghailiang@huawei.com> writes: >> >>> From: Xiangyou Xie <xiexiangyou@huawei.com> >>> >>> If VM is configured with large size of hugepage, when startup, >>> it will consume lots of time to zero the hugepage memory in the function >>> 'os_mem_prealloc'. >>> Libvirtd will wait 30 seconds for qemu to establish the monitor, >>> If the timeout triggers, libvirtd will send TERM signal to kill qemu. >>> >>> To solve the problem, adjust the processing of '-mon' to the ahead of '-object'. >>> >>> Signed-off-by: Xiangyou Xie <xiexiangyou@huawei.com> >>> Signed-off-by: zhanghailiang <zhang.zhanghailiang@huawei.com> >>> --- >>> vl.c | 8 ++++---- >>> 1 file changed, 4 insertions(+), 4 deletions(-) >>> >>> diff --git a/vl.c b/vl.c >>> index 86bdce0..d0c03fe 100644 >>> --- a/vl.c >>> +++ b/vl.c >>> @@ -4000,6 +4000,10 @@ int main(int argc, char **argv, char **envp) >>> exit(0); >>> } >>> >>> + if (qemu_opts_foreach(qemu_find_opts("mon"), mon_init_func, NULL, 1) != 0) { >>> + exit(1); >>> + } >>> + >>> if (qemu_opts_foreach(qemu_find_opts("object"), >>> object_create, NULL, 0) != 0) { >>> exit(1); >>> @@ -4154,10 +4158,6 @@ int main(int argc, char **argv, char **envp) >>> >>> parse_numa_opts(); >>> >>> - if (qemu_opts_foreach(qemu_find_opts("mon"), mon_init_func, NULL, 1) != 0) { >>> - exit(1); >>> - } >>> - >>> if (foreach_device_config(DEV_SERIAL, serial_parse) < 0) >>> exit(1); >>> if (foreach_device_config(DEV_PARALLEL, parallel_parse) < 0) >> >> >> Errors after monitor initialization look ugly when a monitor is on >> stdio: >> >> $ qemu -nodefaults -monitor stdio -vga xxx >> QEMU 2.2.50 monitor - type 'help' for more information >> (qemu) Unknown vga type: xxx >> $ >> >> This patch initializes monitors earlier, thus makes more errors look >> ugly. Do we care? > > Yeah, this doesn't seem great. Surely the actual problem here > is that we're doing something that takes 30 seconds to initialise > as part of our command line option parsing phase ? Yep. Quoting myself: Our startup is a big happy ad hoc mess. A more organizes [sic] program would read and check configuration first (quick, can fail), then allocate resources (quick, can fail), then initialize (somewhat slow, failure unlikely). Reorganizing everything to work like that is a huge task. Until we get that (if ever!), all we can do is delay the most egregiously slow initializations until after configuration checking and monitor creation.
On 6 March 2015 at 21:05, Markus Armbruster <armbru@redhat.com> wrote: > Peter Maydell <peter.maydell@linaro.org> writes: > >> On 5 March 2015 at 02:02, Markus Armbruster <armbru@redhat.com> wrote: >>> This patch initializes monitors earlier, thus makes more errors look >>> ugly. Do we care? >> >> Yeah, this doesn't seem great. Surely the actual problem here >> is that we're doing something that takes 30 seconds to initialise >> as part of our command line option parsing phase ? > > Yep. Quoting myself: > > Our startup is a big happy ad hoc mess. A more organizes [sic] > program would read and check configuration first (quick, can fail), > then allocate resources (quick, can fail), then initialize (somewhat > slow, failure unlikely). > > Reorganizing everything to work like that is a huge task. > > Until we get that (if ever!), all we can do is delay the most > egregiously slow initializations until after configuration checking and > monitor creation. Yep, so we should delay the memset() til after all our init and option parsing, rather than just move the monitor parsing/init early. -- PMM
diff --git a/vl.c b/vl.c index 86bdce0..d0c03fe 100644 --- a/vl.c +++ b/vl.c @@ -4000,6 +4000,10 @@ int main(int argc, char **argv, char **envp) exit(0); } + if (qemu_opts_foreach(qemu_find_opts("mon"), mon_init_func, NULL, 1) != 0) { + exit(1); + } + if (qemu_opts_foreach(qemu_find_opts("object"), object_create, NULL, 0) != 0) { exit(1); @@ -4154,10 +4158,6 @@ int main(int argc, char **argv, char **envp) parse_numa_opts(); - if (qemu_opts_foreach(qemu_find_opts("mon"), mon_init_func, NULL, 1) != 0) { - exit(1); - } - if (foreach_device_config(DEV_SERIAL, serial_parse) < 0) exit(1); if (foreach_device_config(DEV_PARALLEL, parallel_parse) < 0)