| Message ID | 1366978377-16823-10-git-send-email-dingel@linux.vnet.ibm.com |
|---|---|
| State | New |
| Headers | show |
On 26.04.2013, at 14:12, Dominik Dingel wrote: > From: Christian Paro <cparo@us.ibm.com> > > Provide a loadparm property which can be used to pass IPL load > parameters on a per-device basis for VirtioCcwDevice instances > representing block and network devices. > > Signed-off-by: Christian Paro <cparo@us.ibm.com> > > diff --git a/hw/s390x/virtio-ccw.c b/hw/s390x/virtio-ccw.c > index 56539d3..23d042f 100644 > --- a/hw/s390x/virtio-ccw.c > +++ b/hw/s390x/virtio-ccw.c > @@ -791,6 +791,7 @@ static const VirtIOBindings virtio_ccw_bindings = { > > static Property virtio_ccw_net_properties[] = { > DEFINE_PROP_STRING("devno", VirtioCcwDevice, bus_id), > + DEFINE_PROP_UINT32("loadparm", VirtioCcwDevice, loadparm, 0), We don't support netboot yet, right? Also is "loadparm" an architected name? Wouldn't "bootmap_id" or something that actually tells the user what's going on fit better? Alex > DEFINE_VIRTIO_NET_FEATURES(VirtioCcwDevice, host_features[0]), > DEFINE_VIRTIO_NET_PROPERTIES(VirtIONetCcw, vdev.net_conf), > DEFINE_NIC_PROPERTIES(VirtIONetCcw, vdev.nic_conf), > @@ -818,6 +819,7 @@ static const TypeInfo virtio_ccw_net = { > > static Property virtio_ccw_blk_properties[] = { > DEFINE_PROP_STRING("devno", VirtioCcwDevice, bus_id), > + DEFINE_PROP_UINT32("loadparm", VirtioCcwDevice, loadparm, 0), > DEFINE_VIRTIO_BLK_FEATURES(VirtioCcwDevice, host_features[0]), > DEFINE_VIRTIO_BLK_PROPERTIES(VirtIOBlkCcw, blk), > DEFINE_PROP_END_OF_LIST(), > diff --git a/hw/s390x/virtio-ccw.h b/hw/s390x/virtio-ccw.h > index 84055e7..a3f706a 100644 > --- a/hw/s390x/virtio-ccw.h > +++ b/hw/s390x/virtio-ccw.h > @@ -76,6 +76,7 @@ struct VirtioCcwDevice { > SubchDev *sch; > VirtIODevice *vdev; > char *bus_id; > + uint32_t loadparm; > uint32_t host_features[VIRTIO_CCW_FEATURE_SIZE]; > VirtIORNGConf rng; > VirtioBusState bus; > -- > 1.7.9.5 >
On Fri, 26 Apr 2013 18:52:48 +0200 Alexander Graf <agraf@suse.de> wrote: > > On 26.04.2013, at 14:12, Dominik Dingel wrote: > > > From: Christian Paro <cparo@us.ibm.com> > > > > Provide a loadparm property which can be used to pass IPL load > > parameters on a per-device basis for VirtioCcwDevice instances > > representing block and network devices. > > > > Signed-off-by: Christian Paro <cparo@us.ibm.com> > > > > diff --git a/hw/s390x/virtio-ccw.c b/hw/s390x/virtio-ccw.c > > index 56539d3..23d042f 100644 > > --- a/hw/s390x/virtio-ccw.c > > +++ b/hw/s390x/virtio-ccw.c > > @@ -791,6 +791,7 @@ static const VirtIOBindings virtio_ccw_bindings = { > > > > static Property virtio_ccw_net_properties[] = { > > DEFINE_PROP_STRING("devno", VirtioCcwDevice, bus_id), > > + DEFINE_PROP_UINT32("loadparm", VirtioCcwDevice, loadparm, 0), > > We don't support netboot yet, right? Also is "loadparm" an architected name? Wouldn't "bootmap_id" or something that actually tells the user what's going on fit better? > > > Alex Christian had the idea to name it subindex, but I don't know, to be honest, I like the general open solution more, especially for the commandline interface. What if we later on want to change the behaviour, loadparm, or better biosparameter is so general... where bootmap_id is narrowed down. Dominik > > DEFINE_VIRTIO_NET_FEATURES(VirtioCcwDevice, host_features[0]), > > DEFINE_VIRTIO_NET_PROPERTIES(VirtIONetCcw, vdev.net_conf), > > DEFINE_NIC_PROPERTIES(VirtIONetCcw, vdev.nic_conf), > > @@ -818,6 +819,7 @@ static const TypeInfo virtio_ccw_net = { > > > > static Property virtio_ccw_blk_properties[] = { > > DEFINE_PROP_STRING("devno", VirtioCcwDevice, bus_id), > > + DEFINE_PROP_UINT32("loadparm", VirtioCcwDevice, loadparm, 0), > > DEFINE_VIRTIO_BLK_FEATURES(VirtioCcwDevice, host_features[0]), > > DEFINE_VIRTIO_BLK_PROPERTIES(VirtIOBlkCcw, blk), > > DEFINE_PROP_END_OF_LIST(), > > diff --git a/hw/s390x/virtio-ccw.h b/hw/s390x/virtio-ccw.h > > index 84055e7..a3f706a 100644 > > --- a/hw/s390x/virtio-ccw.h > > +++ b/hw/s390x/virtio-ccw.h > > @@ -76,6 +76,7 @@ struct VirtioCcwDevice { > > SubchDev *sch; > > VirtIODevice *vdev; > > char *bus_id; > > + uint32_t loadparm; > > uint32_t host_features[VIRTIO_CCW_FEATURE_SIZE]; > > VirtIORNGConf rng; > > VirtioBusState bus; > > -- > > 1.7.9.5 > > >
On 26.04.2013, at 20:08, Dominik Dingel wrote: > On Fri, 26 Apr 2013 18:52:48 +0200 > Alexander Graf <agraf@suse.de> wrote: > >> >> On 26.04.2013, at 14:12, Dominik Dingel wrote: >> >>> From: Christian Paro <cparo@us.ibm.com> >>> >>> Provide a loadparm property which can be used to pass IPL load >>> parameters on a per-device basis for VirtioCcwDevice instances >>> representing block and network devices. >>> >>> Signed-off-by: Christian Paro <cparo@us.ibm.com> >>> >>> diff --git a/hw/s390x/virtio-ccw.c b/hw/s390x/virtio-ccw.c >>> index 56539d3..23d042f 100644 >>> --- a/hw/s390x/virtio-ccw.c >>> +++ b/hw/s390x/virtio-ccw.c >>> @@ -791,6 +791,7 @@ static const VirtIOBindings virtio_ccw_bindings = { >>> >>> static Property virtio_ccw_net_properties[] = { >>> DEFINE_PROP_STRING("devno", VirtioCcwDevice, bus_id), >>> + DEFINE_PROP_UINT32("loadparm", VirtioCcwDevice, loadparm, 0), >> >> We don't support netboot yet, right? Also is "loadparm" an architected name? Wouldn't "bootmap_id" or something that actually tells the user what's going on fit better? >> >> >> Alex > > Christian had the idea to name it subindex, but I don't know, to be honest, I like the general open solution more, especially for the commandline interface. What if we later on want to change the behaviour, loadparm, or better biosparameter is so general... where bootmap_id is narrowed down. How about you guys make up your mind first what you really want to describe and which use cases you want to cover? :) Alex
On 26/04/13 20:14, Alexander Graf wrote: > > On 26.04.2013, at 20:08, Dominik Dingel wrote: > >> On Fri, 26 Apr 2013 18:52:48 +0200 >> Alexander Graf <agraf@suse.de> wrote: >> >>> >>> On 26.04.2013, at 14:12, Dominik Dingel wrote: >>> >>>> From: Christian Paro <cparo@us.ibm.com> >>>> >>>> Provide a loadparm property which can be used to pass IPL load >>>> parameters on a per-device basis for VirtioCcwDevice instances >>>> representing block and network devices. >>>> >>>> Signed-off-by: Christian Paro <cparo@us.ibm.com> >>>> >>>> diff --git a/hw/s390x/virtio-ccw.c b/hw/s390x/virtio-ccw.c >>>> index 56539d3..23d042f 100644 >>>> --- a/hw/s390x/virtio-ccw.c >>>> +++ b/hw/s390x/virtio-ccw.c >>>> @@ -791,6 +791,7 @@ static const VirtIOBindings virtio_ccw_bindings = { >>>> >>>> static Property virtio_ccw_net_properties[] = { >>>> DEFINE_PROP_STRING("devno", VirtioCcwDevice, bus_id), >>>> + DEFINE_PROP_UINT32("loadparm", VirtioCcwDevice, loadparm, 0), >>> >>> We don't support netboot yet, right? Also is "loadparm" an architected name? Wouldn't "bootmap_id" or something that actually tells the user what's going on fit better? >>> >>> >>> Alex >> >> Christian had the idea to name it subindex, but I don't know, to be honest, I like the general open solution more, especially for the commandline interface. What if we later on want to change the behaviour, loadparm, or better biosparameter is so general... where bootmap_id is narrowed down. You should probably specify the Christian.. ;-) My vote was for loadparm, because thats how the parameter is called in HMC and in z/VM as well as under /sys/firmware/reipl/ccw/loadparm etc. On the other hand its only loadparm for ccw disk devices. fcp has wwpn,lun etc. Its getting late in the cycle, so lets focus on the bootindex and defer this after 1.5.
diff --git a/hw/s390x/virtio-ccw.c b/hw/s390x/virtio-ccw.c index 56539d3..23d042f 100644 --- a/hw/s390x/virtio-ccw.c +++ b/hw/s390x/virtio-ccw.c @@ -791,6 +791,7 @@ static const VirtIOBindings virtio_ccw_bindings = { static Property virtio_ccw_net_properties[] = { DEFINE_PROP_STRING("devno", VirtioCcwDevice, bus_id), + DEFINE_PROP_UINT32("loadparm", VirtioCcwDevice, loadparm, 0), DEFINE_VIRTIO_NET_FEATURES(VirtioCcwDevice, host_features[0]), DEFINE_VIRTIO_NET_PROPERTIES(VirtIONetCcw, vdev.net_conf), DEFINE_NIC_PROPERTIES(VirtIONetCcw, vdev.nic_conf), @@ -818,6 +819,7 @@ static const TypeInfo virtio_ccw_net = { static Property virtio_ccw_blk_properties[] = { DEFINE_PROP_STRING("devno", VirtioCcwDevice, bus_id), + DEFINE_PROP_UINT32("loadparm", VirtioCcwDevice, loadparm, 0), DEFINE_VIRTIO_BLK_FEATURES(VirtioCcwDevice, host_features[0]), DEFINE_VIRTIO_BLK_PROPERTIES(VirtIOBlkCcw, blk), DEFINE_PROP_END_OF_LIST(), diff --git a/hw/s390x/virtio-ccw.h b/hw/s390x/virtio-ccw.h index 84055e7..a3f706a 100644 --- a/hw/s390x/virtio-ccw.h +++ b/hw/s390x/virtio-ccw.h @@ -76,6 +76,7 @@ struct VirtioCcwDevice { SubchDev *sch; VirtIODevice *vdev; char *bus_id; + uint32_t loadparm; uint32_t host_features[VIRTIO_CCW_FEATURE_SIZE]; VirtIORNGConf rng; VirtioBusState bus;