Am 24.04.2013 um 17:32 hat Pavel Hrdina geschrieben:
> Signed-off-by: Pavel Hrdina <phrd...@redhat.com>
> --- a/block/sheepdog.c
> +++ b/block/sheepdog.c
> @@ -1867,7 +1867,9 @@ cleanup:
> return ret;
> }
>
> -static int sd_snapshot_goto(BlockDriverState *bs, const char *snapshot_id)
> +static void sd_snapshot_goto(BlockDriverState *bs,
> + const char *snapshot_id,
> + Error **errp)
> {
> BDRVSheepdogState *s = bs->opaque;
> BDRVSheepdogState *old_s;
> @@ -1892,13 +1894,13 @@ static int sd_snapshot_goto(BlockDriverState *bs,
> const char *snapshot_id)
>
> ret = find_vdi_name(s, vdi, snapid, tag, &vid, 1);
> if (ret) {
> - error_report("Failed to find_vdi_name");
> + error_setg_errno(errp, -ret, "Failed to find VDI snapshot '%s'",
> vdi);
Isn't snapid what identifies the snapshot? Or maybe the combination of
vdi and snapid.
> goto out;
> }
>
> fd = connect_to_sdog(s);
> if (fd < 0) {
> - ret = fd;
> + error_setg_errno(errp, -fd, "Failed to connect to sdog");
> goto out;
> }
>
> @@ -1909,14 +1911,14 @@ static int sd_snapshot_goto(BlockDriverState *bs,
> const char *snapshot_id)
> closesocket(fd);
>
> if (ret) {
> + error_setg_errno(errp, -ret, "Failed to open VDI snapshot '%s'",
> vdi);
> goto out;
> }
>
> memcpy(&s->inode, buf, sizeof(s->inode));
>
> if (!s->inode.vm_state_size) {
> - error_report("Invalid snapshot");
> - ret = -ENOENT;
> + error_setg(errp, "Invalid snapshot '%s'", snapshot_id);
Here as well.
> goto out;
> }
>
> @@ -1925,16 +1927,12 @@ static int sd_snapshot_goto(BlockDriverState *bs,
> const char *snapshot_id)
> g_free(buf);
> g_free(old_s);
>
> - return 0;
> + return;
> out:
> /* recover bdrv_sd_state */
> memcpy(s, old_s, sizeof(BDRVSheepdogState));
> g_free(buf);
> g_free(old_s);
> -
> - error_report("failed to open. recover old bdrv_sd_state.");
> -
> - return ret;
> }
>
> static void sd_snapshot_delete(BlockDriverState *bs,
> --- a/savevm.c
> +++ b/savevm.c
> @@ -2529,11 +2529,11 @@ int load_vmstate(const char *name)
> bs = NULL;
> while ((bs = bdrv_next(bs))) {
> if (bdrv_can_snapshot(bs)) {
> - ret = bdrv_snapshot_goto(bs, name);
> - if (ret < 0) {
> - error_report("Error %d while activating snapshot '%s' on
> '%s'",
> - ret, name, bdrv_get_device_name(bs));
> - return ret;
> + bdrv_snapshot_goto(bs, name, &local_err);
> + if (error_is_set(&local_err)) {
> + qerror_report_err(local_err);
Shouldn't we add the device name on which the failure occurred?
> + error_free(local_err);
> + return -EIO;
> }
> }
> }
Kevin
