From patchwork Mon Mar 26 13:37:28 2012 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Anthony Liguori X-Patchwork-Id: 148742 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Received: from lists.gnu.org (lists.gnu.org [208.118.235.17]) (using TLSv1 with cipher AES256-SHA (256/256 bits)) (Client did not present a certificate) by ozlabs.org (Postfix) with ESMTPS id 903ACB6EEC for ; Tue, 27 Mar 2012 00:37:54 +1100 (EST) Received: from localhost ([::1]:52851 helo=lists.gnu.org) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1SCA7M-0005Xx-3U for incoming@patchwork.ozlabs.org; Mon, 26 Mar 2012 09:37:52 -0400 Received: from eggs.gnu.org ([208.118.235.92]:34053) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1SCA7A-0005XO-Qe for Qemu-devel@nongnu.org; Mon, 26 Mar 2012 09:37:46 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1SCA73-0000b9-D6 for Qemu-devel@nongnu.org; Mon, 26 Mar 2012 09:37:40 -0400 Received: from mail-ob0-f173.google.com ([209.85.214.173]:62095) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1SCA73-0000ak-8b for Qemu-devel@nongnu.org; Mon, 26 Mar 2012 09:37:33 -0400 Received: by obbwd20 with SMTP id wd20so6026314obb.4 for ; Mon, 26 Mar 2012 06:37:31 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=20120113; h=message-id:date:from:user-agent:mime-version:to:cc:subject :references:in-reply-to:content-type:content-transfer-encoding :x-gm-message-state; bh=NTdrDuCbLg88VPtMfjS5rn0WazryBNoUTOIOXIAsyII=; b=k4q0oJYtivrmQDKHAdkez4L8NHHMzyq+0LzXO+VZw6RiFVlz+LlV4Z2SUeBkRK/8mj 4Jd9m+8oolXPtuXWf3XYuyRoKK8IRVBfmbgDdNG26C24kNBwltwj2VHUaObh4oMeOUpk olwbzaCmEpZGeTndVsNMeSjSBClbsPGqDkVqBqI0YRhxQhr6aAtn3qg6WLY3ukhbeyrb FgDqfkCym3NLd/zYDsE+FR7GqqrsQ6mm7+uHTGcgaG4qC3hHQVdSndvgZNZ4g3ELdrty RLy0UBEXs2gfBHV7unHZ1kKzkWQsV58X7LxKsCXtICMLfOispC6AIkKnJxxxXhZapegO FxNA== Received: by 10.182.222.34 with SMTP id qj2mr2584826obc.27.1332769051042; Mon, 26 Mar 2012 06:37:31 -0700 (PDT) Received: from [192.168.0.108] (cpe-70-123-132-139.austin.res.rr.com. [70.123.132.139]) by mx.google.com with ESMTPS id w4sm16415536obx.2.2012.03.26.06.37.29 (version=TLSv1/SSLv3 cipher=OTHER); Mon, 26 Mar 2012 06:37:29 -0700 (PDT) Message-ID: <4F707118.1070200@codemonkey.ws> Date: Mon, 26 Mar 2012 08:37:28 -0500 From: Anthony Liguori User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:11.0) Gecko/20120310 Thunderbird/11.0 MIME-Version: 1.0 To: Kevin Wolf References: <4F702B56.8030400@redhat.com> In-Reply-To: <4F702B56.8030400@redhat.com> X-Gm-Message-State: ALoCoQmWDfOlD0GzRX+fKttk4weCDhs5zvJ507SKU5A9NcSGtjkXDed49JKDHTs9NMAbfK0Y9IZP X-detected-operating-system: by eggs.gnu.org: Genre and OS details not recognized. X-Received-From: 209.85.214.173 Cc: Qemu-devel@nongnu.org, Luiz Capitulino Subject: Re: [Qemu-devel] Ignoring errno makes QMP errors suck X-BeenThere: qemu-devel@nongnu.org X-Mailman-Version: 2.1.14 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: qemu-devel-bounces+incoming=patchwork.ozlabs.org@nongnu.org Sender: qemu-devel-bounces+incoming=patchwork.ozlabs.org@nongnu.org On 03/26/2012 03:39 AM, Kevin Wolf wrote: > Hi, > > I keep getting reports of problems, with nice error descriptions that > usually look very similar to what I produced here: > > {"execute":"blockdev-snapshot-sync","arguments":{"device":"ide0-hd0","snapshot-file":"/tmp/backing.qcow2"}} > {"error": {"class": "OpenFileFailed", "desc": "Could not open > '/tmp/backing.qcow2'", "data": {"filename": "/tmp/backing.qcow2"}}} This is not QMP's fault. This is the block layers. Specifically, you're missing: > been the helpful additional information in this case) > > How should management tools ever be able to provide a helpful error > message to their users if all they get is this useless "something went > wrong" error? You need to kill off error_report in the block layer and replace it with error_set. The problem with error_report is that while you can understand what "Unknown file format 'qcow2'" means, management tools can't. Responding that "the tool can just present that error to the user" implies that the management tool only provides an English-language interface which is not terribly friendly. QMP provides all the infrastructure you need. You just have to use it. Regards, Anthony Liguori > Kevin diff --git a/blockdev.c b/blockdev.c index 1a500b8..04c3a39 100644 --- a/blockdev.c +++ b/blockdev.c @@ -777,7 +777,11 @@ void qmp_transaction(BlockdevActionList *dev_list, Error ** states->old_bs->drv->format_name, NULL, -1, flags); if (ret) { - error_set(errp, QERR_OPEN_FILE_FAILED, new_image_file); + if (ret == -EPERM) { + error_set(errp, QERR_PERMISSION_DENIED); + } else { + error_set(errp, QERR_OPEN_FILE_FAILED, new_image_file); + } goto delete_and_fail; } } Which is handling: ret = bdrv_img_create(new_image_file, format, states->old_bs->filename, states->old_bs->drv->format_name, NULL, -1, flags); But it would be even better to push Error ** into bdrv_img_create(). There's only two callers so it would be trivial to do that. Then you could do: diff --git a/block.c b/block.c index b88ee90..a7bf8a9 100644 --- a/block.c +++ b/block.c @@ -3881,7 +3881,8 @@ bdrv_acct_done(BlockDriverState *bs, BlockAcctCookie *cook int bdrv_img_create(const char *filename, const char *fmt, const char *base_filename, const char *base_fmt, - char *options, uint64_t img_size, int flags) + char *options, uint64_t img_size, int flags, + Error **errp) { QEMUOptionParameter *param = NULL, *create_options = NULL; QEMUOptionParameter *backing_fmt, *backing_file, *size; @@ -3893,14 +3894,14 @@ int bdrv_img_create(const char *filename, const char *fm /* Find driver and parse its options */ drv = bdrv_find_format(fmt); if (!drv) { - error_report("Unknown file format '%s'", fmt); + error_set(errp, QERR_INVALID_BLOCK_FORMAT, fmt); ret = -EINVAL; goto out; } Etc. > Who can tell me what has happened here? Oh, yes, the command failed, I > would have guessed that from the "error" key. But the actual error > description is as useless as it gets. It doesn't tell me anything about > _why_ the snapshot couldn't be created. ("Permission denied" would have