On 一, 2011-12-12 at 19:15 +0800, Kevin Wolf wrote: > Am 12.12.2011 11:58, schrieb Pekka Enberg: > > On Mon, 12 Dec 2011, Kevin Wolf wrote: > >>> @@ -667,14 +722,11 @@ static struct qcow_refcount_block *qcow_read_refcount_block(struct qcow *q, u64 > >>> > >>> rft_idx = clust_idx >> (header->cluster_bits - QCOW_REFCOUNT_BLOCK_SHIFT); > >>> if (rft_idx >= rft->rf_size) > >>> - return NULL; > >>> + return (void *)-ENOSPC; > >> > >> Is this allowed style in kvm-tool? :-/ > > > > It needs to use ERR_PTR() and related macros but otherwise I don't see a > > big problem with it. > > Can you be sure that it never clashes with a valid allocation when you > use this in userspace? > > But yes, at least using appropriate functions should be required. And > this means that you can't only check for -ENOSPC, but you need to check > for all possible error codes (IS_ERR_VALUE() I guess). The qcow_read_refcount_block() is invoiked in the two places qcow_get_refcount() and update_cluster_refcount() and will only return NULL or -ENOSPC. In the qcow_get_refcount(), when qcow_read_refcount_block() returned -ENOSPEC(no refcount block is available.), returning zero which means the cluster is not in use and the refcount block can be grew in the update_cluster_refcount(). In the update_cluster_refcount(), when qcow_read_refcount_block() returned -ENOSPEC, it is necessary to grow the refcount blocks. So the ENOSPEC should be specially dealt with. Does this make sense? :) > Kevin Thanks Tianyu Lan. -- To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
