Re: [PATCH parted] dasd: Fix ped_disk_new_fresh() to not read old data from disk (rh533808)

[Hans de Goede]

Hi,

On 11/09/2009 06:28 PM, Jim Meyering wrote:
> Hans de Goede wrote:
> > dasd_write(), was reading the volume label from the disk (trough
> > fdasd_check_volume()) and later writing it back again, this is fine for
> > existing dasd labels, but when creating a fresh label, this would
> > also cause the old volume label to be re-used, and if the old label
> > was corrupt, it would cause fdasd_check_volume() and thus dasd_write()
> > to fail.
> >
> > * libparted/arch/linux.c (toplevel): include fdasd.h.
> > (init_dasd): Do BIODASDINFO ioctl, and store the dasd devno in
> > arch_specific.
> > (init_dasd): Remove dead (never reached) code.
> > * libparted/arch/linux.h (struct _LinuxSpecific): Add devno member.
> > * libparted/labels/dasd.c (DasdDiskSpecific): add vlabel member.
> > (dasd_alloc): Init DasdDiskSpecific.vlabel for fresh disks
> > (dasd_read): Store read vlabel in DasdDiskSpecific.vlabel.
> > (dasd_write): Write DasdDiskSpecific.vlabel instead of on disk vlabel.
>
> Thanks!
> I'll do a thorough review tomorrow,
> but in the mean time, would you please write a sentence about
> this in NEWS?

One NEWS patch coming up after this reply :)

> Also, I want to be able to exercise this fix.
> Assuming I have an s390 VM, I suppose I could
> create a dasd partition table from a dd-created file
> full of zeros, corrupt part of it, and then what?

You don't even need to corrupt it, just do:
dd if=/dev/zero of=/dev/dasdX bs=1024 count=500

And then call ped_disk_new_fresh(device, dasdDiskType), and
then call ped_disk_commit_to_dev on the resulting disk and it
will fail, as the fdasd_check_volume() call in dasd_write()
will fail, as it reads the volume label (which is a small part
of a total dasd label) from the disk, gets all zero's which is
not valid and fails.

Regards,

Hans