Re: [Qemu-devel] [PATCH v2] virtio-blk: add default serial id

[Dave Young]

On Fri, Sep 21, 2012 at 08:15:38AM -0600, Eric Blake wrote:
> On 09/21/2012 07:30 AM, Dave Young wrote:
> > 
> > For virtio block device, if user does not specify the serial attribute,
> > There will be no serial availabe, this is not convenient for identifying
> > the disk.
> > 
> > Doing something similar to ide disks, add a "VD0000?" default serial
> > number if user does not specify it.
> > 
> > [v1->v2 address comments from Eric Blake]:
> > fix spell errors in patch description
> > decrease drive_serial in virtio_blk_exit as well
> 
> Typically, patch changelogs belong...
> 
> > 
> > Signed-off-by: Dave Young <address@hidden>
> > ---
> 
> ...after the --- line, so that 'git am' doesn't make them part of git
> history.  Also, I'm not sure that decreasing the serial number is
> correct - you've now made it much easier to get duplicate serial numbers
> compared to my original complaint of 100000 hotplug cycles.  Now all I
> have to do is:
> 
> create a guest with two disks
> hot unplug disk one
> hot plug a new disk
> 
> and voila, both disks will now have serial number 2.

Thanks for comment.

Add changelogs to git history is not bad IMO, this can reflect the changes
between diffrent version of the patches, it's quite normal. 

For the serial number decreasing issue, I think there's only these two ways to
select, there's no ideal way to resolve this issue.
My use case for this is for the kdump kernel to find proper disks,
after 1st kernel crashing 2nd kernel need find right disk to dump vmcore.
In this case v1 and v2 aproaches are both find to me.

>From my point of view, patch v1 is better though, I think unpluging 100000 is
not a sane use case. It's not likely to happen.

> 
> > @@ -632,6 +638,7 @@ VirtIODevice *virtio_blk_init(DeviceStat
> >                                            sizeof(struct virtio_blk_config),
> >                                            sizeof(VirtIOBlock));
> >  
> > +    s->drive_serial = drive_serial++;
> >      s->vdev.get_config = virtio_blk_update_config;
> >      s->vdev.set_config = virtio_blk_set_config;
> >      s->vdev.get_features = virtio_blk_get_features;
> > @@ -664,4 +671,5 @@ void virtio_blk_exit(VirtIODevice *vdev)
> >      unregister_savevm(s->qdev, "virtio-blk", s);
> >      blockdev_mark_auto_del(s->bs);
> >      virtio_cleanup(vdev);
> > +    drive_serial--;
> >  }
> > 
> > 
> 
> -- 
> Eric Blake   address@hidden    +1-919-301-3266
> Libvirt virtualization library http://libvirt.org
>