On Fri, Aug 29, 2008 at 10:27:04AM -0400, Cole Robinson wrote: > Jim Meyering wrote: > > Cole Robinson <crobinso@xxxxxxxxxx> wrote: > > ... > >> Okay, second cut attached. I followed your recommendations: > > ... > > > > With all of Dan's comments addressed, this should be fine. > > > >> diff --git a/src/domain_conf.c b/src/domain_conf.c > > ... > >> +int virDiskNameToBusDeviceIndex(virDomainDiskDefPtr disk, > > ... > >> + switch (disk->bus) { > >> + case VIR_DOMAIN_DISK_BUS_IDE: > >> + *busIdx = ((idx - (idx % 2)) / 2); > >> + *devIdx = (idx % 2); > >> + break; > >> + case VIR_DOMAIN_DISK_BUS_SCSI: > >> + *busIdx = ((idx - (idx % 7)) / 7); > >> + *devIdx = (idx % 7); > >> + break; > > > > It doesn't affect correctness, but you can remove the > > unnecessary "- (idx % 2)" and "- (idx % 7)" expressions: > > > > switch (disk->bus) { > > case VIR_DOMAIN_DISK_BUS_IDE: > > *busIdx = idx / 2; > > *devIdx = idx % 2; > > break; > > case VIR_DOMAIN_DISK_BUS_SCSI: > > *busIdx = idx / 7; > > *devIdx = idx % 7; > > break; > > Okay, latest cut. This addresses the above piece pointed out > by Jim and all of Dan's comments. ACK from me now. Daniel -- |: Red Hat, Engineering, London -o- http://people.redhat.com/berrange/ :| |: http://libvirt.org -o- http://virt-manager.org -o- http://ovirt.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: GnuPG: 7D3B9505 -o- F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 :| -- Libvir-list mailing list Libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list