Re: [PATCH 4/5] KVM: s390: Add a channel I/O based virtio transport driver.

Cornelia Huck
2012-10-30T13:03:04Z

On Mon, 29 Oct 2012 19:37:10 +0100
Alexander Graf <agraf@suse.de> wrote:

>
> On 29.10.2012, at 19:34, Cornelia Huck wrote:
>
> > On Mon, 29 Oct 2012 19:12:54 +0100
> > Alexander Graf <agraf@suse.de> wrote:
> >
> >>
> >> On 29.10.2012, at 14:07, Cornelia Huck wrote:
> >
> >>> +static void virtio_ccw_kvm_notify(struct virtqueue *vq)
> >>> +{
> >>> + struct virtio_ccw_vq_info *info = vq->priv;
> >>> + struct virtio_ccw_device *vcdev;
> >>> + struct subchannel_id schid;
> >>> + __u32 reg2;
> >>> +
> >>> + vcdev = to_vc_device(info->vq->vdev);
> >>> + ccw_device_get_schid(vcdev->cdev, &schid);
> >>> + reg2 = *(__u32 *)&schid;
> >>
> >> That cast looks quite ugly. Can't you just access the field in there you need? Or if it's multiple fields do a union over them? Or assemble them by hand in C?
> >
> > I think the cast looks less ugly than using a union to morph it around.
> > I want the schid with all fields filled out anyway, since this is what
> > identifies the subchannel.
>
> How about a helper function that returns a u32 for a struct subchannel_id in arch/s390/include/asm/schid.h then?

This would just move the cast around, no? I don't think that would
improve readability.