Re: [PATCH 10/14] kvm tools: Rename raw_image_ops to blk_dev_ops

[Ingo Molnar <mingo@elte.hu>]

* Asias He <asias.hejun@gmail.com> wrote:

> -	fd		= open(filename, O_RDONLY);
> +	/*
> +	 * Be careful! We are opening host block device!
> +	 * Open it readonly since we do not want to break user's data on disk.
> +	 */
> +	fd			= open(filename, O_RDONLY);
>  	if (fd < 0)
>  		return NULL;

btw., this is a repeating pattern i noticed: you align assignment vertically 
even if it's a stand-alone assignment.

We want to apply vertical alignment only when it helps readability - and 
repeated assingments such as:

        *job = (struct thread_pool__job) {
                .kvm            = kvm,
                .data           = data,
                .callback       = callback,
                .mutex          = PTHREAD_MUTEX_INITIALIZER
        };

indeed look *much* better when aligned vertically. Same goes for structure 
definitions. Thanks for applying those concepts uniformly around tools/kvm/,
it makes the code visibly more pleasant to read.

But the above standalone assignment of 'fd' does not seem to be such a case: 
the right side of the assignment just 'floats' freely in space with no other 
similar assingment next to it giving it structure.

Thus the old-fashioned:

	fd = open(filename, O_RDONLY);
 	if (fd < 0)
 		return NULL;

is a lot more readable form IMO.

There's many similar examples of isolated assignments looking weird, all around 
the code.

Thanks,

	Ingo