Re: [libvirt] [Patch]Fix bugs of Sheepdog storage driver
by harryxiyou
On Fri, Feb 8, 2013 at 1:06 PM, Osier Yang <jyang(a)redhat.com> wrote:
[...]
> Google Groups小组敬上
>
> That's bad if one could get one notice like this each time after
> reviewing your patch. :-) Not sure how to get rid of this, but
> I think it's caused by the permission, so please either change
> the google group to public, or don't cc to the list when posting
> patch.
>
Thanks, it's because i add "cloudxy(a)googlegroups.com" ML to the
CC'ed list.
>> You sent same patch multiple times. It might be caused by the mail
>> delay though, I.E, you don't see the one sent out earlier. Perhaps
>> you should check you mailbox setting to avoid the small flood.
>>
Sorry, i am not familiar with 'git send-email --compose' well. So it sends
several times. I will fix this problem.
[...]
>> So, consider to change the commit log to:
>>
>> Don't try to refresh the volume if creating volume fails.
This one is well for me, thanks.
>>>
>>> Signed-off-by: Harry Wei<harryxiyou(a)gmail.com>
>>> ---
>>> src/storage/storage_backend_sheepdog.c | 5 ++++-
>>> 1 files changed, 4 insertions(+), 1 deletions(-)
>>>
>>> diff --git a/src/storage/storage_backend_sheepdog.c
>>> b/src/storage/storage_backend_sheepdog.c
>>> index cd18f33..f987604 100644
>>> --- a/src/storage/storage_backend_sheepdog.c
>>> +++ b/src/storage/storage_backend_sheepdog.c
>>> @@ -168,9 +168,12 @@ virStorageBackendSheepdogCreateVol(virConnectPtr
>>> conn ATTRIBUTE_UNUSED,
>>> virCommandAddArgFormat(cmd, "%llu", vol->capacity);
>>> virStorageBackendSheepdogAddHostArg(cmd, pool);
>>> ret = virCommandRun(cmd, NULL);
>>> + if (ret< 0)
>>> + goto cleanup;
>>
>>
>> We prefer:
>>
>> if ((ret = virCommandRun(cmd, NULL)) < 0)
>> goto cleanup;
>>
>> However, you can avoid using the "ret" here by initialize it
>> to "-1", with that you can simply do:
>>
>> if (virCommandRun(cmd, NULL) < 0)
>> goto cleanup;
>>
>>>
>>> - virStorageBackendSheepdogRefreshVol(conn, pool, vol);
>>> + ret = virStorageBackendSheepdogRefreshVol(conn, pool, vol);
>>
>>
>> And:
>>
>> if (virStorageBackendSheepdogRefreshVol(conn, pool, vol) < 0)
>> goto cleanup;
>>
>> ret = 0;
>>
>>>
>>> +cleanup:
>>> virCommandFree(cmd);
>>> return ret;
>>> }
We cannot do like
[...]
if (virCommandRun(cmd, NULL) < 0)
goto cleanup;
[...]
if (virStorageBackendSheepdogRefreshVol(conn, pool, vol) < 0)
goto cleanup;
[...]
+cleanup:
virCommandFree(cmd);
return ret;
We must initialize "ret", because if either virCommandRun and
virStorageBackendSheepdogRefreshVol has some errors, they
will return '-1' to "ret". Then execute "return ret", which who calls
virStorageBackendSheepdogCreateVol will know it has happened
some errors. So we should modify them like this.
[...]
if ((ret = virCommandRun(cmd, NULL)) < 0)
goto cleanup;
[...]
if (ret = virStorageBackendSheepdogRefreshVol(conn, pool, vol)) < 0)
goto cleanup;
cleanup:
virCommandFree(cmd);
return ret;
}
Osier Yang, do you agree with me?
--
Thanks
Harry Wei
12 years, 2 months
[libvirt] [PATCH 0/3] Make access to virCapsPtr threadsafe in QEMU
by Daniel P. Berrange
The 'caps' object in virQEMUDriverPtr can be re-created at any
time, so currently any access is protected by the 'driver'
lock. This is really bad for concurrency, since the 'caps'
object is used all over the place. This series refactors the
QEMU driver so that this is a reference counted object. The
lock is now only required while obtaining a reference to the
object. This will let us remove the QEMU driver lock from much
code in the future.
12 years, 2 months
[libvirt] [Patch]Fix bugs of Sheepdog storage driver
by harryxiyou＠gmail.com
virStorageBackendSheepdogCreateVol calls virCommandRun whihout checking
the return value, which this return value determines us whether we should
call virStorageBackendSheepdogRefreshVol func. And we should also give a
return value to virStorageBackendSheepdogRefreshVol func because it may
cause errors. Following patch can fix these bugs. This patch have checked
by 'make syntax-check'.
Signed-off-by: Harry Wei <harryxiyou(a)gmail.com>
---
src/storage/storage_backend_sheepdog.c | 5 ++++-
1 files changed, 4 insertions(+), 1 deletions(-)
diff --git a/src/storage/storage_backend_sheepdog.c b/src/storage/storage_backend_sheepdog.c
index cd18f33..f987604 100644
--- a/src/storage/storage_backend_sheepdog.c
+++ b/src/storage/storage_backend_sheepdog.c
@@ -168,9 +168,12 @@ virStorageBackendSheepdogCreateVol(virConnectPtr conn ATTRIBUTE_UNUSED,
virCommandAddArgFormat(cmd, "%llu", vol->capacity);
virStorageBackendSheepdogAddHostArg(cmd, pool);
ret = virCommandRun(cmd, NULL);
+ if (ret < 0)
+ goto cleanup;
- virStorageBackendSheepdogRefreshVol(conn, pool, vol);
+ ret = virStorageBackendSheepdogRefreshVol(conn, pool, vol);
+cleanup:
virCommandFree(cmd);
return ret;
}
--
1.7.0.4
12 years, 2 months
[libvirt] [Patch]Fix bugs of Sheepdog storage driver
by harryxiyou＠gmail.com
diff --git a/src/storage/storage_backend_sheepdog.c b/src/storage/storage_backend_sheepdog.c
index cd18f33..f987604 100644
--- a/src/storage/storage_backend_sheepdog.c
+++ b/src/storage/storage_backend_sheepdog.c
@@ -168,9 +168,12 @@ virStorageBackendSheepdogCreateVol(virConnectPtr conn ATTRIBUTE_UNUSED,
virCommandAddArgFormat(cmd, "%llu", vol->capacity);
virStorageBackendSheepdogAddHostArg(cmd, pool);
ret = virCommandRun(cmd, NULL);
+ if (ret < 0)
+ goto cleanup;
- virStorageBackendSheepdogRefreshVol(conn, pool, vol);
+ ret = virStorageBackendSheepdogRefreshVol(conn, pool, vol);
+cleanup:
virCommandFree(cmd);
return ret;
}
--
1.7.0.4
12 years, 2 months
[libvirt] [Patch]Fix a bug of Sheepdog storage driver
by harryxiyou＠gmail.com
Signed-off-by: Harry Wei <harryxiyou(a)gmail.com>
---
src/storage/storage_backend_sheepdog.c | 5 ++++-
1 files changed, 4 insertions(+), 1 deletions(-)
diff --git a/src/storage/storage_backend_sheepdog.c b/src/storage/storage_backend_sheepdog.c
index cd18f33..f987604 100644
--- a/src/storage/storage_backend_sheepdog.c
+++ b/src/storage/storage_backend_sheepdog.c
@@ -168,9 +168,12 @@ virStorageBackendSheepdogCreateVol(virConnectPtr conn ATTRIBUTE_UNUSED,
virCommandAddArgFormat(cmd, "%llu", vol->capacity);
virStorageBackendSheepdogAddHostArg(cmd, pool);
ret = virCommandRun(cmd, NULL);
+ if (ret < 0)
+ goto cleanup;
- virStorageBackendSheepdogRefreshVol(conn, pool, vol);
+ ret = virStorageBackendSheepdogRefreshVol(conn, pool, vol);
+cleanup:
virCommandFree(cmd);
return ret;
}
--
1.7.0.4
12 years, 2 months
[libvirt] [PATCH v2 0/5] cgroup refactor
by Hu Tao
This series refactors cgroup code to:
- provide lazy creation of cgroup directories, despite of what
level they are.
- remove cgroup directories if no one is using the corresponding
virCgroup.
This series is splitted in a manner for easier review. Patch 2 is
the main implementation. Patch 3, 4, 5 should have been squashed
into one in order to build, but harder to review.
Hu Tao (5):
refactor virCgroupDetectMounts and virCgroupDetectPlacement
cgroup: refactor virCgroup
cgroup: replace old cgroup
qemu: replace old cgroup
lxc: replace old cgroup
src/conf/domain_conf.h | 5 +
src/libvirt_private.syms | 7 +-
src/lxc/lxc_cgroup.c | 40 +-
src/lxc/lxc_cgroup.h | 2 +-
src/lxc/lxc_controller.c | 31 +-
src/lxc/lxc_driver.c | 177 +++----
src/lxc/lxc_process.c | 19 +-
src/qemu/qemu_cgroup.c | 162 +++---
src/qemu/qemu_cgroup.h | 3 +-
src/qemu/qemu_driver.c | 351 +++++--------
src/qemu/qemu_hotplug.c | 21 +-
src/qemu/qemu_migration.c | 20 +-
src/qemu/qemu_process.c | 7 +-
src/util/vircgroup.c | 1263 +++++++++++++++++++++------------------------
src/util/vircgroup.h | 19 +-
15 files changed, 921 insertions(+), 1206 deletions(-)
--
1.8.0.1.240.ge8a1f5a
12 years, 2 months
[libvirt] [PATCH 00/13] Resolve resource leaks found by Valgrind
by John Ferlan
This set of patches resolves issues seen in the Valgrind tests.
The update to the .valgrind.supp file is from my environment and is
very basic. Some paths in the previous version don't exist anymore.
Based on recent (and continuing) commandtest failures, adjustments
still may be necessary depending on environment.
To assist in that I also updated the hacking document to describe a bit
more about the Valgrind tests with respect to success, failure, and
false positives. In particular, how to hopefully recognize each and
decide whether what's seen is a false positive.
Prior to the recent vircommand and commandtest changes, the commandtest
was passing, but now it fails. The failure may be related to:
https://www.redhat.com/archives/libvir-list/2013-February/msg00269.html
I spent some time looking at the output, but could not figure out a
root cause. I'll keep trying, but figured at the very least the other
changes are still valid.
John Ferlan (13):
virnettlscontexttest: Resolve memory leak found by Valgrind
qemuxml2argvtest: Resolve resource leaks found by Valgrind
netdev_vlan_conf: Resolve memory leak found by Valgrind.
vport_profile_conf: Resolve memory leak found by Valgrind
domain_conf: Resolve resource leaks found by Valgrind
qemu_command: Resolve resource leaks found by Valgrind
qemumonitorjsontest: Resolve resource leaks found by Valgrind
qemumonitortestutils: Resolve resource leaks found by Valgrind
virnetttlcontext: Resolve issues found by Valgrind
valgrind: Adjust the suppression file
hacking: Add more information about Valgrind
HACKING: Sync with docs/hacking.html.in
cfg.mk: Add hacking.in.html to sc_prohibit_raw_allocation
HACKING | 93 ++++++++++-
cfg.mk | 2 +-
docs/hacking.html.in | 96 +++++++++++-
src/conf/domain_conf.c | 12 +-
src/conf/netdev_vlan_conf.c | 3 +-
src/conf/netdev_vport_profile_conf.c | 1 +
src/qemu/qemu_command.c | 68 ++++----
src/rpc/virnettlscontext.c | 4 +-
tests/.valgrind.supp | 292 +++++++----------------------------
tests/qemumonitorjsontest.c | 23 ++-
tests/qemumonitortestutils.c | 7 +-
tests/qemuxml2argvtest.c | 6 +-
tests/virnettlscontexttest.c | 8 +-
13 files changed, 319 insertions(+), 296 deletions(-)
--
1.7.11.7
12 years, 2 months
[libvirt] [PATCH 2/2] Add syntax-check to make sure Python files don't contain trailing semicolon
by Guido Günther
We allow for a trailing semicolon in full line comments since
docs/index.py has some SQL statements in it.
---
cfg.mk | 7 +++++++
1 file changed, 7 insertions(+)
diff --git a/cfg.mk b/cfg.mk
index 2dfde01..b92ae85 100644
--- a/cfg.mk
+++ b/cfg.mk
@@ -702,6 +702,13 @@ sc_require_enum_last_marker:
{ echo '$(ME): enum impl needs to use _LAST marker' 1>&2; \
exit 1; } || :
+# In Python files we don't want to end lines with a semicolon like in C
+sc_prohibit_semicolon_at_eol_in_python:
+ @prohibit='^[^#].*\;$$' \
+ in_vc_files='\.py$$' \
+ halt="Don't use semicolon at eol in python files" \
+ $(_sc_search_regexp)
+
# We don't use this feature of maint.mk.
prev_version_file = /dev/null
--
1.7.10.4
12 years, 2 months
[libvirt] [PATCH] syntax-check: Don't check non-reentrant functions in docs
by Peter Krempa
Otherwise constructions like "random (used by default)" end up breaking
syntax check by apparently using non-reentrant functions.
---
cfg.mk | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/cfg.mk b/cfg.mk
index 2dfde01..309a2d7 100644
--- a/cfg.mk
+++ b/cfg.mk
@@ -813,7 +813,7 @@ exclude_file_name_regexp--sc_prohibit_newline_at_end_of_diagnostic = \
^src/rpc/gendispatch\.pl$$
exclude_file_name_regexp--sc_prohibit_nonreentrant = \
- ^((po|tests)/|docs/.*py|run.in$$)
+ ^((po|tests)/|docs/.*(py|html\.in)|run.in$$)
exclude_file_name_regexp--sc_prohibit_raw_allocation = \
^(src/util/viralloc\.[ch]|examples/.*|tests/securityselinuxhelper.c)$$
--
1.8.1.1
12 years, 2 months
[libvirt] [PATCH] qemu: Report nicer error if decompression of save image fails
by Peter Krempa
This patch is a little more specific what failed while trying to restore
a guest from a save image.
---
src/qemu/qemu_driver.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index 979a027..c96ba7a 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -4903,6 +4903,8 @@ qemuDomainSaveImageStartVM(virConnectPtr conn,
if (virCommandWait(cmd, NULL) < 0) {
qemuProcessStop(driver, vm, VIR_DOMAIN_SHUTOFF_FAILED, 0);
+ virReportError(VIR_ERR_OPERATION_FAILED, "%s",
+ _("Decompression of save image failed"));
ret = -1;
}
VIR_DEBUG("Decompression binary stderr: %s", NULLSTR(errbuf));
--
1.8.1.1
12 years, 2 months
