Re: [Qemu-devel] [PATCH 1/6] bootindex: add {del, modify}_boot_device_path function

[Gonglei (Arei)]

Hi, Gerd

> -----Original Message-----
> From: Gerd Hoffmann [mailto:address@hidden
> Sent: Thursday, July 24, 2014 6:09 PM
> To: Gonglei (Arei)
> Cc: address@hidden; address@hidden; address@hidden;
> address@hidden; address@hidden; address@hidden;
> address@hidden; address@hidden; address@hidden;
> address@hidden; address@hidden; address@hidden;
> address@hidden; address@hidden; address@hidden;
> address@hidden; address@hidden; address@hidden;
> address@hidden; address@hidden; address@hidden;
> Huangweidong (C); Luonengjun; Huangpeng (Peter); chenliang (T)
> Subject: Re: [PATCH 1/6] bootindex: add {del,modify}_boot_device_path
> function
> 
> On Do, 2014-07-24 at 16:38 +0800, address@hidden wrote:
> > From: Gonglei <address@hidden>
> >
> > When we want to change one device's bootindex, we should do three
> > things. On the on hand, remove the device from global fw_boot_order list,
> > regardless attaching suffix or not delete. On the other hand, delete
> > original object of the assigned bootindex. Finally add the new device's
> > bootindex into the global fw_boot_order list.
> 
> Hmm.  I think we should simply lookup the device and modify the
> bootindex, leaving the entry as-is otherwise.  In case the new bootindex
> is already used by another device just throw an error.
> 
If we just throw an error but not change the bootindex is already used,
we cannot achieve our purpose. For example, we configure a hard disk,
which bootindex=1, a nic which bootindex=2. If we want to boot the guest
from nic firstly, we should must set the nic's bootindex to 1. AFAICT, the 
bootindex=1 always be used.
 
> > +void del_boot_device_path(int32_t bootindex, DeviceState *dev,
> > +                          const char *suffix);
> 
> Should be del_boot_device_path(DeviceState *dev) and simply delete all
> entries belonging to the device.  Patch #3 can be much simpler then as
> we can call the function from generic device cleanup code.
> 
Because the IDE device may configure two kind of disk, HD and CDROM, we
have to distinguish them by suffix.

> > +void modify_boot_device_path(int32_t bootindex, DeviceState *dev,
> > +                             const char *suffix);
> 
> No need for suffix here if we just update the existing entry.
> 
> cheers,
>   Gerd
> 

Best regards,
-Gonglei