On Mon 02 Nov 2015 05:11:46 PM CET, Eric Blake <[email protected]> wrote: >> @@ -1667,6 +1667,11 @@ static void >> external_snapshot_prepare(BlkTransactionState *common, >> >> if (state->new_bs->backing != NULL) { >> error_setg(errp, "The snapshot already has a backing image"); >> + return; >> + } >> + >> + if (!state->new_bs->drv->supports_backing) { >> + error_setg(errp, "The snapshot does not support backing images"); >> } >> } > > You could avoid the 'return' by doing: > > if (state->new_bs->backing) { > error_setg(...); > } else if (!state->new_bs->drv->supports_backing) { > error_setg(...); > }
There's three more checks immediately before these lines, so I would have had to turn everything into a series of if / else if, which is a bit more unreadable in this case in my opinion. That's why I decided to leave it like it is in the patch. Berto
