10:12 a.m.
Jim pointed out some places where the openvz driver could deference some
NULs, so this patch fixes them
Daniel
Index: src/openvz_driver.c
===================================================================
RCS file: /data/cvs/libvirt/src/openvz_driver.c,v
retrieving revision 1.46
diff -u -p -r1.46 openvz_driver.c
--- src/openvz_driver.c 5 Sep 2008 15:00:14 -0000 1.46
+++ src/openvz_driver.c 5 Sep 2008 15:11:34 -0000
@@ -276,7 +276,7 @@ static char *openvzDomainDumpXML(virDoma
static int openvzDomainShutdown(virDomainPtr dom) {
struct openvz_driver *driver = (struct openvz_driver *)dom->conn->privateData;
virDomainObjPtr vm = virDomainFindByUUID(driver->domains, dom->uuid);
- const char *prog[] = {VZCTL, "--quiet", "stop",
vm->def->name, NULL};
+ const char *prog[] = {VZCTL, "--quiet", "stop", NULL /* name */,
NULL};
if (!vm) {
openvzError(dom->conn, VIR_ERR_INVALID_DOMAIN,
@@ -290,6 +290,7 @@ static int openvzDomainShutdown(virDomai
return -1;
}
+ prog[3] = vm->def->name;
if (virRun(dom->conn, prog, NULL) < 0)
return -1;
@@ -303,7 +304,7 @@ static int openvzDomainReboot(virDomainP
unsigned int flags ATTRIBUTE_UNUSED) {
struct openvz_driver *driver = (struct openvz_driver *)dom->conn->privateData;
virDomainObjPtr vm = virDomainFindByUUID(driver->domains, dom->uuid);
- const char *prog[] = {VZCTL, "--quiet", "restart",
vm->def->name, NULL};
+ const char *prog[] = {VZCTL, "--quiet", "restart", NULL /* name
*/, NULL};
if (!vm) {
openvzError(dom->conn, VIR_ERR_INVALID_DOMAIN,
@@ -317,6 +318,7 @@ static int openvzDomainReboot(virDomainP
return -1;
}
+ prog[3] = vm->def->name;
if (virRun(dom->conn, prog, NULL) < 0)
return -1;
@@ -581,7 +583,7 @@ openvzDomainCreate(virDomainPtr dom)
{
struct openvz_driver *driver = (struct openvz_driver *)dom->conn->privateData;
virDomainObjPtr vm = virDomainFindByName(driver->domains, dom->name);
- const char *prog[] = {VZCTL, "--quiet", "start",
vm->def->name, NULL };
+ const char *prog[] = {VZCTL, "--quiet", "start", NULL /* name */,
NULL };
if (!vm) {
openvzError(dom->conn, VIR_ERR_INVALID_DOMAIN,
@@ -595,6 +597,7 @@ openvzDomainCreate(virDomainPtr dom)
return -1;
}
+ prog[3] = vm->def->name;
if (virRun(dom->conn, prog, NULL) < 0) {
openvzError(dom->conn, VIR_ERR_INTERNAL_ERROR,
_("Could not exec %s"), VZCTL);
@@ -614,7 +617,7 @@ openvzDomainUndefine(virDomainPtr dom)
virConnectPtr conn= dom->conn;
struct openvz_driver *driver = (struct openvz_driver *) conn->privateData;
virDomainObjPtr vm = virDomainFindByUUID(driver->domains, dom->uuid);
- const char *prog[] = { VZCTL, "--quiet", "destroy",
vm->def->name, NULL };
+ const char *prog[] = { VZCTL, "--quiet", "destroy", NULL /* name
*/, NULL };
if (!vm) {
openvzError(conn, VIR_ERR_INVALID_DOMAIN, _("no domain with matching
uuid"));
@@ -626,6 +629,7 @@ openvzDomainUndefine(virDomainPtr dom)
return -1;
}
+ prog[3] = vm->def->name;
if (virRun(conn, prog, NULL) < 0) {
openvzError(conn, VIR_ERR_INTERNAL_ERROR,
_("Could not exec %s"), VZCTL);
@@ -643,7 +647,7 @@ openvzDomainSetAutostart(virDomainPtr do
virConnectPtr conn= dom->conn;
struct openvz_driver *driver = (struct openvz_driver *) conn->privateData;
virDomainObjPtr vm = virDomainFindByUUID(driver->domains, dom->uuid);
- const char *prog[] = { VZCTL, "--quiet", "set",
vm->def->name,
+ const char *prog[] = { VZCTL, "--quiet", "set", NULL /* name */,
"--onboot", autostart ? "yes" :
"no",
"--save", NULL };
@@ -652,6 +656,7 @@ openvzDomainSetAutostart(virDomainPtr do
return -1;
}
+ prog[3] = vm->def->name;
if (virRun(conn, prog, NULL) < 0) {
openvzError(conn, VIR_ERR_INTERNAL_ERROR, _("Could not exec %s"),
VZCTL);
return -1;
@@ -704,16 +709,9 @@ static int openvzDomainSetVcpus(virDomai
struct openvz_driver *driver = (struct openvz_driver *) conn->privateData;
virDomainObjPtr vm = virDomainFindByUUID(driver->domains, dom->uuid);
char str_vcpus[32];
- const char *prog[] = { VZCTL, "--quiet", "set",
vm->def->name,
+ const char *prog[] = { VZCTL, "--quiet", "set", NULL /* name */,
"--cpus", str_vcpus, "--save", NULL };
- snprintf(str_vcpus, 31, "%d", nvcpus);
- str_vcpus[31] = '\0';
- if (nvcpus <= 0) {
- openvzError(conn, VIR_ERR_INTERNAL_ERROR,
- _("VCPUs should be >= 1"));
- return -1;
- }
if (!vm) {
openvzError(conn, VIR_ERR_INVALID_DOMAIN,
@@ -721,6 +719,16 @@ static int openvzDomainSetVcpus(virDomai
return -1;
}
+ if (nvcpus <= 0) {
+ openvzError(conn, VIR_ERR_INTERNAL_ERROR,
+ _("VCPUs should be >= 1"));
+ return -1;
+ }
+
+ snprintf(str_vcpus, 31, "%d", nvcpus);
+ str_vcpus[31] = '\0';
+
+ prog[3] = vm->def->name;
if (virRun(conn, prog, NULL) < 0) {
openvzError(conn, VIR_ERR_INTERNAL_ERROR,
_("Could not exec %s"), VZCTL);
--
|: Red Hat, Engineering, London -o- http://people.redhat.com/berrange/ :|
|: http://libvirt.org -o- http://virt-manager.org -o- http://ovirt.org :|
|: http://autobuild.org -o- http://search.cpan.org/~danberr/ :|
|: GnuPG: 7D3B9505 -o- F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 :|
10:40 a.m.
"Daniel P. Berrange" <berrange(a)redhat.com> wrote:
Jim pointed out some places where the openvz driver could deference
some
NULs, so this patch fixes them
...
Index: src/openvz_driver.c
===================================================================
RCS file: /data/cvs/libvirt/src/openvz_driver.c,v
retrieving revision 1.46
diff -u -p -r1.46 openvz_driver.c
--- src/openvz_driver.c 5 Sep 2008 15:00:14 -0000 1.46
+++ src/openvz_driver.c 5 Sep 2008 15:11:34 -0000
@@ -276,7 +276,7 @@ static char *openvzDomainDumpXML(virDoma
static int openvzDomainShutdown(virDomainPtr dom) {
struct openvz_driver *driver = (struct openvz_driver
*)dom->conn->privateData;
virDomainObjPtr vm = virDomainFindByUUID(driver->domains, dom->uuid);
- const char *prog[] = {VZCTL, "--quiet", "stop",
vm->def->name, NULL};
+ const char *prog[] = {VZCTL, "--quiet", "stop", NULL /* name */,
NULL};
...
+ prog[3] = vm->def->name;
if (virRun(dom->conn, prog, NULL) < 0)
return -1;
@@ -303,7 +304,7 @@ static int openvzDomainReboot(virDomainP
unsigned int flags ATTRIBUTE_UNUSED) {
struct openvz_driver *driver = (struct openvz_driver
*)dom->conn->privateData;
virDomainObjPtr vm = virDomainFindByUUID(driver->domains, dom->uuid);
- const char *prog[] = {VZCTL, "--quiet", "restart",
vm->def->name, NULL};
+ const char *prog[] = {VZCTL, "--quiet", "restart", NULL /* name
*/, NULL};
if (!vm) {
openvzError(dom->conn, VIR_ERR_INVALID_DOMAIN,
@@ -317,6 +318,7 @@ static int openvzDomainReboot(virDomainP
return -1;
}
+ prog[3] = vm->def->name;
...
@@ -643,7 +647,7 @@ openvzDomainSetAutostart(virDomainPtr do
virConnectPtr conn= dom->conn;
struct openvz_driver *driver = (struct openvz_driver *) conn->privateData;
virDomainObjPtr vm = virDomainFindByUUID(driver->domains, dom->uuid);
- const char *prog[] = { VZCTL, "--quiet", "set",
vm->def->name,
+ const char *prog[] = { VZCTL, "--quiet", "set", NULL /* name
*/,
"--onboot", autostart ? "yes" :
"no",
"--save", NULL };
@@ -652,6 +656,7 @@ openvzDomainSetAutostart(virDomainPtr do
return -1;
}
+ prog[3] = vm->def->name;
...
All looks correct.
However, I'm a little nervous about hard-coding those '[3]'s.
What if someone inserts a new --foo option somewhere before
the NULL place-holder, or otherwise rearranges the options?
Then we'll have to remember to update that "3" below to e.g.,
"4".
Easy to miss that...
What do you think about putting a marker string like "NAME"
in place of your new NULL, and then iterate through the array
of strings searching for the marker; once found, put
vm->def->name in that slot; else fail.
There are enough uses (6) that it might be worthwhile.
4:46 a.m.
On Fri, Sep 05, 2008 at 05:40:16PM +0200, Jim Meyering wrote:
"Daniel P. Berrange" <berrange(a)redhat.com> wrote:
> Jim pointed out some places where the openvz driver could deference some
> NULs, so this patch fixes them
...
> Index: src/openvz_driver.c
> ===================================================================
> RCS file: /data/cvs/libvirt/src/openvz_driver.c,v
> retrieving revision 1.46
> diff -u -p -r1.46 openvz_driver.c
> --- src/openvz_driver.c 5 Sep 2008 15:00:14 -0000 1.46
> +++ src/openvz_driver.c 5 Sep 2008 15:11:34 -0000
> @@ -276,7 +276,7 @@ static char *openvzDomainDumpXML(virDoma
> static int openvzDomainShutdown(virDomainPtr dom) {
> struct openvz_driver *driver = (struct openvz_driver
*)dom->conn->privateData;
> virDomainObjPtr vm = virDomainFindByUUID(driver->domains, dom->uuid);
> - const char *prog[] = {VZCTL, "--quiet", "stop",
vm->def->name, NULL};
> + const char *prog[] = {VZCTL, "--quiet", "stop", NULL /*
name */, NULL};
...
> + prog[3] = vm->def->name;
> if (virRun(dom->conn, prog, NULL) < 0)
> return -1;
>
...
All looks correct.
However, I'm a little nervous about hard-coding those '[3]'s.
What if someone inserts a new --foo option somewhere before
the NULL place-holder, or otherwise rearranges the options?
Then we'll have to remember to update that "3" below to e.g.,
"4".
Easy to miss that...
I just realized there is a much simpler way todo this...
Daniel
Index: src/openvz_driver.c
===================================================================
RCS file: /data/cvs/libvirt/src/openvz_driver.c,v
retrieving revision 1.46
diff -u -p -r1.46 openvz_driver.c
--- src/openvz_driver.c 5 Sep 2008 15:00:14 -0000 1.46
+++ src/openvz_driver.c 8 Sep 2008 09:44:53 -0000
@@ -276,7 +276,7 @@ static char *openvzDomainDumpXML(virDoma
static int openvzDomainShutdown(virDomainPtr dom) {
struct openvz_driver *driver = (struct openvz_driver *)dom->conn->privateData;
virDomainObjPtr vm = virDomainFindByUUID(driver->domains, dom->uuid);
- const char *prog[] = {VZCTL, "--quiet", "stop",
vm->def->name, NULL};
+ const char *prog[] = {VZCTL, "--quiet", "stop", vm ?
vm->def->name : NULL, NULL};
if (!vm) {
openvzError(dom->conn, VIR_ERR_INVALID_DOMAIN,
@@ -303,7 +303,7 @@ static int openvzDomainReboot(virDomainP
unsigned int flags ATTRIBUTE_UNUSED) {
struct openvz_driver *driver = (struct openvz_driver *)dom->conn->privateData;
virDomainObjPtr vm = virDomainFindByUUID(driver->domains, dom->uuid);
- const char *prog[] = {VZCTL, "--quiet", "restart",
vm->def->name, NULL};
+ const char *prog[] = {VZCTL, "--quiet", "restart", vm ?
vm->def->name : NULL, NULL};
if (!vm) {
openvzError(dom->conn, VIR_ERR_INVALID_DOMAIN,
@@ -581,7 +581,7 @@ openvzDomainCreate(virDomainPtr dom)
{
struct openvz_driver *driver = (struct openvz_driver *)dom->conn->privateData;
virDomainObjPtr vm = virDomainFindByName(driver->domains, dom->name);
- const char *prog[] = {VZCTL, "--quiet", "start",
vm->def->name, NULL };
+ const char *prog[] = {VZCTL, "--quiet", "start", vm ?
vm->def->name : NULL, NULL };
if (!vm) {
openvzError(dom->conn, VIR_ERR_INVALID_DOMAIN,
@@ -614,7 +614,7 @@ openvzDomainUndefine(virDomainPtr dom)
virConnectPtr conn= dom->conn;
struct openvz_driver *driver = (struct openvz_driver *) conn->privateData;
virDomainObjPtr vm = virDomainFindByUUID(driver->domains, dom->uuid);
- const char *prog[] = { VZCTL, "--quiet", "destroy",
vm->def->name, NULL };
+ const char *prog[] = { VZCTL, "--quiet", "destroy", vm ?
vm->def->name : NULL, NULL };
if (!vm) {
openvzError(conn, VIR_ERR_INVALID_DOMAIN, _("no domain with matching
uuid"));
@@ -643,7 +643,7 @@ openvzDomainSetAutostart(virDomainPtr do
virConnectPtr conn= dom->conn;
struct openvz_driver *driver = (struct openvz_driver *) conn->privateData;
virDomainObjPtr vm = virDomainFindByUUID(driver->domains, dom->uuid);
- const char *prog[] = { VZCTL, "--quiet", "set",
vm->def->name,
+ const char *prog[] = { VZCTL, "--quiet", "set", vm ?
vm->def->name : NULL,
"--onboot", autostart ? "yes" :
"no",
"--save", NULL };
@@ -704,16 +704,9 @@ static int openvzDomainSetVcpus(virDomai
struct openvz_driver *driver = (struct openvz_driver *) conn->privateData;
virDomainObjPtr vm = virDomainFindByUUID(driver->domains, dom->uuid);
char str_vcpus[32];
- const char *prog[] = { VZCTL, "--quiet", "set",
vm->def->name,
+ const char *prog[] = { VZCTL, "--quiet", "set", vm ?
vm->def->name : NULL,
"--cpus", str_vcpus, "--save", NULL };
- snprintf(str_vcpus, 31, "%d", nvcpus);
- str_vcpus[31] = '\0';
- if (nvcpus <= 0) {
- openvzError(conn, VIR_ERR_INTERNAL_ERROR,
- _("VCPUs should be >= 1"));
- return -1;
- }
if (!vm) {
openvzError(conn, VIR_ERR_INVALID_DOMAIN,
@@ -721,6 +714,15 @@ static int openvzDomainSetVcpus(virDomai
return -1;
}
+ if (nvcpus <= 0) {
+ openvzError(conn, VIR_ERR_INTERNAL_ERROR,
+ _("VCPUs should be >= 1"));
+ return -1;
+ }
+
+ snprintf(str_vcpus, 31, "%d", nvcpus);
+ str_vcpus[31] = '\0';
+
if (virRun(conn, prog, NULL) < 0) {
openvzError(conn, VIR_ERR_INTERNAL_ERROR,
_("Could not exec %s"), VZCTL);
--
|: Red Hat, Engineering, London -o- http://people.redhat.com/berrange/ :|
|: http://libvirt.org -o- http://virt-manager.org -o- http://ovirt.org :|
|: http://autobuild.org -o- http://search.cpan.org/~danberr/ :|
|: GnuPG: 7D3B9505 -o- F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 :|
4:51 a.m.
"Daniel P. Berrange" <berrange(a)redhat.com> wrote:
On Fri, Sep 05, 2008 at 05:40:16PM +0200, Jim Meyering wrote:
...
> All looks correct.
> However, I'm a little nervous about hard-coding those '[3]'s.
> What if someone inserts a new --foo option somewhere before
> the NULL place-holder, or otherwise rearranges the options?
> Then we'll have to remember to update that "3" below to e.g.,
"4".
> Easy to miss that...
I just realized there is a much simpler way todo this...
...
- const char *prog[] = {VZCTL, "--quiet",
"stop", vm->def->name, NULL};
+ const char *prog[] = {VZCTL, "--quiet", "stop", vm ?
vm->def->name : NULL, NULL};
Ahh... much better ;-)
ACK
5:35 a.m.
On Mon, Sep 08, 2008 at 11:51:28AM +0200, Jim Meyering wrote:
"Daniel P. Berrange" <berrange(a)redhat.com> wrote:
> On Fri, Sep 05, 2008 at 05:40:16PM +0200, Jim Meyering wrote:
...
>> All looks correct.
>> However, I'm a little nervous about hard-coding those '[3]'s.
>> What if someone inserts a new --foo option somewhere before
>> the NULL place-holder, or otherwise rearranges the options?
>> Then we'll have to remember to update that "3" below to e.g.,
"4".
>> Easy to miss that...
>
> I just realized there is a much simpler way todo this...
...
> - const char *prog[] = {VZCTL, "--quiet", "stop",
vm->def->name, NULL};
> + const char *prog[] = {VZCTL, "--quiet", "stop", vm ?
vm->def->name : NULL, NULL};
Ahh... much better ;-)
ACK
Okay, fine by me too !
Daniel
--
Daniel Veillard | libxml Gnome XML XSLT toolkit http://xmlsoft.org/
daniel(a)veillard.com | Rpmfind RPM search engine http://rpmfind.net/
http://veillard.com/ | virtualization library http://libvirt.org/
7:06 a.m.
On Fri, Sep 05, 2008 at 05:40:16PM +0200, Jim Meyering wrote:
> "Daniel P. Berrange" <berrange(a)redhat.com> wrote:
>> Jim pointed out some places where the openvz driver could deference some
>> NULs, so this patch fixes them
> ...
>> Index: src/openvz_driver.c
>> ===================================================================
>> RCS file: /data/cvs/libvirt/src/openvz_driver.c,v
>> retrieving revision 1.46
>> diff -u -p -r1.46 openvz_driver.c
>> --- src/openvz_driver.c 5 Sep 2008 15:00:14 -0000 1.46
>> +++ src/openvz_driver.c 5 Sep 2008 15:11:34 -0000
>> @@ -276,7 +276,7 @@ static char *openvzDomainDumpXML(virDoma
>> static int openvzDomainShutdown(virDomainPtr dom) {
>> struct openvz_driver *driver = (struct openvz_driver
*)dom->conn->privateData;
>> virDomainObjPtr vm = virDomainFindByUUID(driver->domains, dom->uuid);
>> - const char *prog[] = {VZCTL, "--quiet", "stop",
vm->def->name, NULL};
>> + const char *prog[] = {VZCTL, "--quiet", "stop", NULL /*
name */, NULL};
> ...
>> + prog[3] = vm->def->name;
>> if (virRun(dom->conn, prog, NULL) < 0)
>> return -1;
>>
> ...
>
> All looks correct.
> However, I'm a little nervous about hard-coding those '[3]'s.
> What if someone inserts a new --foo option somewhere before
> the NULL place-holder, or otherwise rearranges the options?
> Then we'll have to remember to update that "3" below to e.g.,
"4".
> Easy to miss that...
I just realized there is a much simpler way todo this...
Daniel
Index: src/openvz_driver.c
===================================================================
RCS file: /data/cvs/libvirt/src/openvz_driver.c,v
retrieving revision 1.46
diff -u -p -r1.46 openvz_driver.c
--- src/openvz_driver.c 5 Sep 2008 15:00:14 -0000 1.46
+++ src/openvz_driver.c 8 Sep 2008 09:44:53 -0000
@@ -276,7 +276,7 @@ static char *openvzDomainDumpXML(virDoma
static int openvzDomainShutdown(virDomainPtr dom) {
struct openvz_driver *driver = (struct openvz_driver
*)dom->conn->privateData;
virDomainObjPtr vm = virDomainFindByUUID(driver->domains, dom->uuid);
- const char *prog[] = {VZCTL, "--quiet", "stop",
vm->def->name, NULL};
+ const char *prog[] = {VZCTL, "--quiet", "stop", vm ?
vm->def->name : NULL, NULL};
if (!vm) {
openvzError(dom->conn, VIR_ERR_INVALID_DOMAIN,
@@ -303,7 +303,7 @@ static int openvzDomainReboot(virDomainP
unsigned int flags ATTRIBUTE_UNUSED) {
struct openvz_driver *driver = (struct openvz_driver
*)dom->conn->privateData;
virDomainObjPtr vm = virDomainFindByUUID(driver->domains, dom->uuid);
- const char *prog[] = {VZCTL, "--quiet", "restart",
vm->def->name, NULL};
+ const char *prog[] = {VZCTL, "--quiet", "restart", vm ?
vm->def->name : NULL, NULL};
if (!vm) {
openvzError(dom->conn, VIR_ERR_INVALID_DOMAIN,
@@ -581,7 +581,7 @@ openvzDomainCreate(virDomainPtr dom)
{
struct openvz_driver *driver = (struct openvz_driver
*)dom->conn->privateData;
virDomainObjPtr vm = virDomainFindByName(driver->domains, dom->name);
- const char *prog[] = {VZCTL, "--quiet", "start",
vm->def->name, NULL };
+ const char *prog[] = {VZCTL, "--quiet", "start", vm ?
vm->def->name : NULL, NULL };
if (!vm) {
openvzError(dom->conn, VIR_ERR_INVALID_DOMAIN,
@@ -614,7 +614,7 @@ openvzDomainUndefine(virDomainPtr dom)
virConnectPtr conn= dom->conn;
struct openvz_driver *driver = (struct openvz_driver *) conn->privateData;
virDomainObjPtr vm = virDomainFindByUUID(driver->domains, dom->uuid);
- const char *prog[] = { VZCTL, "--quiet", "destroy",
vm->def->name, NULL };
+ const char *prog[] = { VZCTL, "--quiet", "destroy", vm ?
vm->def->name : NULL, NULL };
if (!vm) {
openvzError(conn, VIR_ERR_INVALID_DOMAIN, _("no domain with matching
uuid"));
@@ -643,7 +643,7 @@ openvzDomainSetAutostart(virDomainPtr do
virConnectPtr conn= dom->conn;
struct openvz_driver *driver = (struct openvz_driver *) conn->privateData;
virDomainObjPtr vm = virDomainFindByUUID(driver->domains, dom->uuid);
- const char *prog[] = { VZCTL, "--quiet", "set",
vm->def->name,
+ const char *prog[] = { VZCTL, "--quiet", "set", vm ?
vm->def->name : NULL,
"--onboot", autostart ? "yes" :
"no",
"--save", NULL };
@@ -704,16 +704,9 @@ static int openvzDomainSetVcpus(virDomai
struct openvz_driver *driver = (struct openvz_driver *) conn->privateData;
virDomainObjPtr vm = virDomainFindByUUID(driver->domains, dom->uuid);
char str_vcpus[32];
- const char *prog[] = { VZCTL, "--quiet", "set",
vm->def->name,
+ const char *prog[] = { VZCTL, "--quiet", "set", vm ?
vm->def->name : NULL,
"--cpus", str_vcpus, "--save", NULL };
- snprintf(str_vcpus, 31, "%d", nvcpus);
- str_vcpus[31] = '\0';
- if (nvcpus <= 0) {
- openvzError(conn, VIR_ERR_INTERNAL_ERROR,
- _("VCPUs should be >= 1"));
- return -1;
- }
if (!vm) {
openvzError(conn, VIR_ERR_INVALID_DOMAIN,
@@ -721,6 +714,15 @@ static int openvzDomainSetVcpus(virDomai
return -1;
}
+ if (nvcpus <= 0) {
+ openvzError(conn, VIR_ERR_INTERNAL_ERROR,
+ _("VCPUs should be >= 1"));
+ return -1;
+ }
+
+ snprintf(str_vcpus, 31, "%d", nvcpus);
+ str_vcpus[31] = '\0';
+
if (virRun(conn, prog, NULL) < 0) {
openvzError(conn, VIR_ERR_INTERNAL_ERROR,
_("Could not exec %s"), VZCTL);
It looks very good.
7:20 a.m.
On Mon, Sep 08, 2008 at 04:06:26PM +0400, Evgeniy Sokolov wrote:
>On Fri, Sep 05, 2008 at 05:40:16PM +0200, Jim Meyering wrote:
>>"Daniel P. Berrange" <berrange(a)redhat.com> wrote:
>>>Jim pointed out some places where the openvz driver could deference some
>>>NULs, so this patch fixes them
>>...
>>>Index: src/openvz_driver.c
>>>===================================================================
>>>RCS file: /data/cvs/libvirt/src/openvz_driver.c,v
>>>retrieving revision 1.46
>>>diff -u -p -r1.46 openvz_driver.c
>>>--- src/openvz_driver.c 5 Sep 2008 15:00:14 -0000 1.46
>>>+++ src/openvz_driver.c 5 Sep 2008 15:11:34 -0000
>>>@@ -276,7 +276,7 @@ static char *openvzDomainDumpXML(virDoma
>>> static int openvzDomainShutdown(virDomainPtr dom) {
>>> struct openvz_driver *driver = (struct openvz_driver
>>> *)dom->conn->privateData;
>>> virDomainObjPtr vm = virDomainFindByUUID(driver->domains,
>>> dom->uuid);
>>>- const char *prog[] = {VZCTL, "--quiet", "stop",
vm->def->name,
>>>NULL};
>>>+ const char *prog[] = {VZCTL, "--quiet", "stop", NULL
/* name */,
>>>NULL};
>>...
>>>+ prog[3] = vm->def->name;
>>> if (virRun(dom->conn, prog, NULL) < 0)
>>> return -1;
>>>
>>...
>>
>>All looks correct.
>>However, I'm a little nervous about hard-coding those '[3]'s.
>>What if someone inserts a new --foo option somewhere before
>>the NULL place-holder, or otherwise rearranges the options?
>>Then we'll have to remember to update that "3" below to e.g.,
"4".
>>Easy to miss that...
>
>I just realized there is a much simpler way todo this...
[snip]
It looks very good.
Thanks, this is committed now
Daniel
--
|: Red Hat, Engineering, London -o- http://people.redhat.com/berrange/ :|
|: http://libvirt.org -o- http://virt-manager.org -o- http://ovirt.org :|
|: http://autobuild.org -o- http://search.cpan.org/~danberr/ :|
|: GnuPG: 7D3B9505 -o- F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 :|
5953
days inactive
5956
days old
6 comments
4 participants
participants (4)
-
Daniel P. Berrange -
Daniel Veillard -
Evgeniy Sokolov -
Jim Meyering