On 05/13/2011 02:10 PM, Cole Robinson wrote:
> Some clients overwrite the error from the regex helper, or do half-baked
> error reporting with the exitstatus.
>
> Signed-off-by: Cole Robinson <crobinso@xxxxxxxxxx>
> ---
> src/storage/storage_backend.c | 11 +++++------
> src/storage/storage_backend.h | 3 +--
> src/storage/storage_backend_fs.c | 5 ++---
> src/storage/storage_backend_iscsi.c | 16 ++--------------
> src/storage/storage_backend_logical.c | 30 +++++-------------------------
> 5 files changed, 15 insertions(+), 50 deletions(-)
>
> - ret = virCommandWait(cmd, outexit);
> + ret = virCommandWait(cmd, NULL);
My only concern with this is that if we were previously being tolerant
of a lousy child program that exits with non-zero status even when
successful, we now fail. But...
> @@ -246,8 +245,8 @@ virStorageBackendFileSystemNetFindPoolSources(virConnectPtr conn ATTRIBUTE_UNUSE
> prog[3] = source->host.name;
>
> if (virStorageBackendRunProgRegex(NULL, prog, 1, regexes, vars,
> - virStorageBackendFileSystemNetFindPoolSourcesFunc,
> - &state, &exitstatus) < 0)
> + virStorageBackendFileSystemNetFindPoolSourcesFunc,
> + &state) < 0)
...this caller was previously ignoring 'showmount' failures, which looks
like it has sane exit status,
> +++ b/src/storage/storage_backend_iscsi.c
> @@ -160,8 +160,7 @@ virStorageBackendISCSISession(virStoragePoolObjPtr pool,
> regexes,
> vars,
> virStorageBackendISCSIExtractSession,
> - &session,
> - NULL) < 0)
> + &session) < 0)
...this caller was already rejecting failures,
> @@ -513,17 +511,7 @@ virStorageBackendISCSIScanTargets(const char *portal,
> regexes,
> vars,
> virStorageBackendISCSIGetTargets,
> - &list,
> - &exitstatus) < 0) {
> - virStorageReportError(VIR_ERR_INTERNAL_ERROR,
> - "%s", _("iscsiadm command failed"));
> - return -1;
> - }
> -
> - if (exitstatus != 0) {
...this caller was manually rejecting errors itself,
> if (virStorageBackendRunProgRegex(pool,
> prog,
> 1,
> regexes,
> vars,
> virStorageBackendLogicalMakeVol,
> - vol,
> - &exitstatus) < 0) {
> - virStorageReportError(VIR_ERR_INTERNAL_ERROR,
> - "%s", _("lvs command failed"));
> - return -1;
> - }
> -
> - if (exitstatus != 0) {
...as was this,
> @@ -331,7 +318,7 @@ virStorageBackendLogicalFindPoolSources(virConnectPtr conn ATTRIBUTE_UNUSED,
> * that might be hanging around, so if this fails for some reason, the
> * worst that happens is that scanning doesn't pick everything up
> */
> - if (virRun(scanprog, &exitstatus) < 0) {
> + if (virRun(scanprog, NULL) < 0) {
> VIR_WARN("Failure when running vgscan to refresh physical volumes");
> }
>
> @@ -339,8 +326,8 @@ virStorageBackendLogicalFindPoolSources(virConnectPtr conn ATTRIBUTE_UNUSED,
> sourceList.type = VIR_STORAGE_POOL_LOGICAL;
>
> if (virStorageBackendRunProgRegex(NULL, prog, 1, regexes, vars,
> - virStorageBackendLogicalFindPoolSourcesFunc,
> - &sourceList, &exitstatus) < 0)
> + virStorageBackendLogicalFindPoolSourcesFunc,
> + &sourceList) < 0)
...another one ignoring failures, but pvs and vgscan look sane,
> @@ -506,13 +492,7 @@ virStorageBackendLogicalRefreshPool(virConnectPtr conn ATTRIBUTE_UNUSED,
> regexes,
> vars,
> virStorageBackendLogicalRefreshPoolFunc,
> - NULL,
> - &exitstatus) < 0) {
> - virStoragePoolObjClearVols(pool);
> - return -1;
> - }
> -
> - if (exitstatus != 0) {
...and one last doing a manual reporting. Looks safe to me, so:
ACK.
--
Eric Blake eblake@xxxxxxxxxx +1-801-349-2682
Libvirt virtualization library http://libvirt.org
Attachment:
signature.asc
Description: OpenPGP digital signature
-- libvir-list mailing list libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list