| Message ID | 4D81BF12.4020500@msgid.tls.msk.ru |
|---|---|
| State | New |
| Headers | show |
Michael Tokarev <mjt@tls.msk.ru> writes: > Trivial patch. I've sent it yesterday but somehow it didn't > reach the list. > > This fixes the problem when qemu continues even if -drive specification > is somehow invalid, resulting in a mess. Applicable for both current > master and for stable-0.14 (and 0.13 and 0.12 as well). Note patch doesn't apply to 0.12 and 0.13. > The prob can actually be seriuos: when you start guest with two drives > and make an error in the specification of one of them, and the guest > has something like a raid array on the two drives, guest may start failing > that array or kick "missing" drives which may result in a mess - this is > what actually happened to me, I did't want a resync at all, and a resync > resulted in re-writing (and allocating) a 4TB virtual drive I used for > testing, which in turn resulted in my filesystem filling up and whole > thing failing badly. Yes it was just testing VM, I experimented with > larger raid arrays, but the end result was quite, well, unexpected. > > Thanks! > > Signed-off-by: Michael Tokarev <mjt@tls.msk.ru> > --- > vl.c | 3 ++- > 1 files changed, 2 insertions(+), 1 deletions(-) > > diff --git a/vl.c b/vl.c > index 8bcf2ae..79f996e 100644 > --- a/vl.c > +++ b/vl.c > @@ -2098,7 +2098,8 @@ int main(int argc, char **argv, char **envp) > HD_OPTS); > break; > case QEMU_OPTION_drive: > - drive_def(optarg); > + if (drive_def(optarg) == NULL) > + exit(1); > break; > case QEMU_OPTION_set: > if (qemu_set_option(optarg) != 0) What about all the other unchecked drive_add() calls in main()?
Am 17.03.2011 08:58, schrieb Michael Tokarev: > Trivial patch. I've sent it yesterday but somehow it didn't > reach the list. > > This fixes the problem when qemu continues even if -drive specification > is somehow invalid, resulting in a mess. Applicable for both current > master and for stable-0.14 (and 0.13 and 0.12 as well). > > The prob can actually be seriuos: when you start guest with two drives > and make an error in the specification of one of them, and the guest > has something like a raid array on the two drives, guest may start failing > that array or kick "missing" drives which may result in a mess - this is > what actually happened to me, I did't want a resync at all, and a resync > resulted in re-writing (and allocating) a 4TB virtual drive I used for > testing, which in turn resulted in my filesystem filling up and whole > thing failing badly. Yes it was just testing VM, I experimented with > larger raid arrays, but the end result was quite, well, unexpected. > > Thanks! > > Signed-off-by: Michael Tokarev <mjt@tls.msk.ru> Before I got a message like this and the guest started anyway: qemu-system-x86_64: -drive asdfj: Invalid parameter 'asdfj' Now it exits like it should, but I don't get an error message any more. Exiting silently isn't really nice either. > --- > vl.c | 3 ++- > 1 files changed, 2 insertions(+), 1 deletions(-) > > diff --git a/vl.c b/vl.c > index 8bcf2ae..79f996e 100644 > --- a/vl.c > +++ b/vl.c > @@ -2098,7 +2098,8 @@ int main(int argc, char **argv, char **envp) > HD_OPTS); > break; > case QEMU_OPTION_drive: > - drive_def(optarg); > + if (drive_def(optarg) == NULL) > + exit(1); Coding style requires braces here. Kevin
Kevin Wolf <kwolf@redhat.com> writes: > Am 17.03.2011 08:58, schrieb Michael Tokarev: >> Trivial patch. I've sent it yesterday but somehow it didn't >> reach the list. >> >> This fixes the problem when qemu continues even if -drive specification >> is somehow invalid, resulting in a mess. Applicable for both current >> master and for stable-0.14 (and 0.13 and 0.12 as well). >> >> The prob can actually be seriuos: when you start guest with two drives >> and make an error in the specification of one of them, and the guest >> has something like a raid array on the two drives, guest may start failing >> that array or kick "missing" drives which may result in a mess - this is >> what actually happened to me, I did't want a resync at all, and a resync >> resulted in re-writing (and allocating) a 4TB virtual drive I used for >> testing, which in turn resulted in my filesystem filling up and whole >> thing failing badly. Yes it was just testing VM, I experimented with >> larger raid arrays, but the end result was quite, well, unexpected. >> >> Thanks! >> >> Signed-off-by: Michael Tokarev <mjt@tls.msk.ru> > > Before I got a message like this and the guest started anyway: > > qemu-system-x86_64: -drive asdfj: Invalid parameter 'asdfj' > > Now it exits like it should, but I don't get an error message any more. [...] Are you sure? I still get the error message in my testing.
Am 17.03.2011 16:33, schrieb Markus Armbruster: > Kevin Wolf <kwolf@redhat.com> writes: > >> Am 17.03.2011 08:58, schrieb Michael Tokarev: >>> Trivial patch. I've sent it yesterday but somehow it didn't >>> reach the list. >>> >>> This fixes the problem when qemu continues even if -drive specification >>> is somehow invalid, resulting in a mess. Applicable for both current >>> master and for stable-0.14 (and 0.13 and 0.12 as well). >>> >>> The prob can actually be seriuos: when you start guest with two drives >>> and make an error in the specification of one of them, and the guest >>> has something like a raid array on the two drives, guest may start failing >>> that array or kick "missing" drives which may result in a mess - this is >>> what actually happened to me, I did't want a resync at all, and a resync >>> resulted in re-writing (and allocating) a 4TB virtual drive I used for >>> testing, which in turn resulted in my filesystem filling up and whole >>> thing failing badly. Yes it was just testing VM, I experimented with >>> larger raid arrays, but the end result was quite, well, unexpected. >>> >>> Thanks! >>> >>> Signed-off-by: Michael Tokarev <mjt@tls.msk.ru> >> >> Before I got a message like this and the guest started anyway: >> >> qemu-system-x86_64: -drive asdfj: Invalid parameter 'asdfj' >> >> Now it exits like it should, but I don't get an error message any more. > [...] > > Are you sure? I still get the error message in my testing. You're right. Instead of using git am to apply the patch, I just edited vl.c manually and probably messed up. The patch works. So it's only the braces. Kevin
17.03.2011 18:04, Kevin Wolf wrote: > Am 17.03.2011 08:58, schrieb Michael Tokarev: [] >> --- a/vl.c >> +++ b/vl.c >> @@ -2098,7 +2098,8 @@ int main(int argc, char **argv, char **envp) >> HD_OPTS); >> break; >> case QEMU_OPTION_drive: >> - drive_def(optarg); >> + if (drive_def(optarg) == NULL) >> + exit(1); > > Coding style requires braces here. I'll just stick it into debian package. I'm not going to change all the other braces like this around that place. Thanks. /mjt
Am 17.03.2011 16:49, schrieb Michael Tokarev: > 17.03.2011 18:04, Kevin Wolf wrote: >> Am 17.03.2011 08:58, schrieb Michael Tokarev: > [] >>> --- a/vl.c >>> +++ b/vl.c >>> @@ -2098,7 +2098,8 @@ int main(int argc, char **argv, char **envp) >>> HD_OPTS); >>> break; >>> case QEMU_OPTION_drive: >>> - drive_def(optarg); >>> + if (drive_def(optarg) == NULL) >>> + exit(1); >> >> Coding style requires braces here. > > I'll just stick it into debian package. I'm not going > to change all the other braces like this around that place. I'm only asking to add one pair of braces in the line that you touch, not to change everything in qemu. Shouldn't be that hard... Kevin
diff --git a/vl.c b/vl.c index 8bcf2ae..79f996e 100644 --- a/vl.c +++ b/vl.c @@ -2098,7 +2098,8 @@ int main(int argc, char **argv, char **envp) HD_OPTS); break; case QEMU_OPTION_drive: - drive_def(optarg); + if (drive_def(optarg) == NULL) + exit(1); break; case QEMU_OPTION_set: if (qemu_set_option(optarg) != 0)
Trivial patch. I've sent it yesterday but somehow it didn't reach the list. This fixes the problem when qemu continues even if -drive specification is somehow invalid, resulting in a mess. Applicable for both current master and for stable-0.14 (and 0.13 and 0.12 as well). The prob can actually be seriuos: when you start guest with two drives and make an error in the specification of one of them, and the guest has something like a raid array on the two drives, guest may start failing that array or kick "missing" drives which may result in a mess - this is what actually happened to me, I did't want a resync at all, and a resync resulted in re-writing (and allocating) a 4TB virtual drive I used for testing, which in turn resulted in my filesystem filling up and whole thing failing badly. Yes it was just testing VM, I experimented with larger raid arrays, but the end result was quite, well, unexpected. Thanks! Signed-off-by: Michael Tokarev <mjt@tls.msk.ru> --- vl.c | 3 ++- 1 files changed, 2 insertions(+), 1 deletions(-)