diff options
| author | Markus Armbruster <armbru@redhat.com> | 2017-03-06 20:00:38 +0100 |
|---|---|---|
| committer | Kevin Wolf <kwolf@redhat.com> | 2017-03-07 14:53:28 +0100 |
| commit | a0dc0e2bfe543404c07258b2a2c4f9d53c0430b1 (patch) | |
| tree | 01d5efe0ee24f964ae050fa07e4b0afa17ecd8b6 | |
| parent | 48d7c4af06393b974b0a515ac9d1cc7346dbae23 (diff) | |
sheepdog: Mark sd_snapshot_delete() lossage FIXME
sd_snapshot_delete() should delete the snapshot whose ID matches
@snapshot_id and whose name matches @name. But that's not what it
does. If @snapshot_id is a valid ID, it deletes the snapshot with
that ID, else it deletes the snapshot with that name. It doesn't use
@name at all. Add suitable FIXME comments, so someone who actually
knows Sheepdog can fix it.
Signed-off-by: Markus Armbruster <armbru@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
| -rw-r--r-- | block/sheepdog.c | 10 |
1 files changed, 10 insertions, 0 deletions
diff --git a/block/sheepdog.c b/block/sheepdog.c index be3db1f150..187bcd8236 100644 --- a/block/sheepdog.c +++ b/block/sheepdog.c @@ -2457,6 +2457,10 @@ static int sd_snapshot_delete(BlockDriverState *bs, const char *name, Error **errp) { + /* + * FIXME should delete the snapshot matching both @snapshot_id and + * @name, but @name not used here + */ unsigned long snap_id = 0; char snap_tag[SD_MAX_VDI_TAG_LEN]; int fd, ret; @@ -2481,6 +2485,11 @@ static int sd_snapshot_delete(BlockDriverState *bs, pstrcpy(buf, SD_MAX_VDI_LEN, s->name); ret = qemu_strtoul(snapshot_id, NULL, 10, &snap_id); if (ret || snap_id > UINT32_MAX) { + /* + * FIXME Since qemu_strtoul() returns -EINVAL when + * @snapshot_id is null, @snapshot_id is mandatory. Correct + * would be to require at least one of @snapshot_id and @name. + */ error_setg(errp, "Invalid snapshot ID: %s", snapshot_id ? snapshot_id : "<null>"); return -EINVAL; @@ -2489,6 +2498,7 @@ static int sd_snapshot_delete(BlockDriverState *bs, if (snap_id) { hdr.snapid = (uint32_t) snap_id; } else { + /* FIXME I suspect we should use @name here */ pstrcpy(snap_tag, sizeof(snap_tag), snapshot_id); pstrcpy(buf + SD_MAX_VDI_LEN, SD_MAX_VDI_TAG_LEN, snap_tag); } |