On Tue, Oct 18, 2022 at 10:15:57AM +0200, Daniel Wagner wrote:
> On Mon, Oct 10, 2022 at 07:15:08PM +0200, Klaus Jensen wrote:
> > This is all upstream. Namespaces with 'shared=on' *should* all be
> > automatically attached to any hotplugged controller devices.
> >
> > With what setup is this not working for you?
>
> Ah okay, I missed the 'shared=on' bit. Let me try again.
Nope, that's not enough. Maybe my setup is not okay?
<qemu:commandline>
<qemu:arg value='-device'/>
<qemu:arg value='pcie-root-port,id=root,slot=1'/>
</qemu:commandline>
qemu-monitor-command tw0 --hmp drive_add 0
if=none,file=/tmp/nvme1.qcow2,format=qcow2,id=nvme1
qemu-monitor-command tw0 --hmp device_add nvme,id=nvme1,serial=nvme-1,bus=root
qemu-monitor-command tw0 --hmp device_add nvme-ns,drive=nvme1,nsid=1,shared=on
Error: Bus 'nvme1' does not support hotplugging
With the patch below the hotplugging works.
diff --git a/hw/nvme/ctrl.c b/hw/nvme/ctrl.c
index 87aeba0564..a09a698873 100644
--- a/hw/nvme/ctrl.c
+++ b/hw/nvme/ctrl.c
@@ -5711,8 +5711,6 @@ static uint16_t nvme_ns_attachment(NvmeCtrl *n,
NvmeRequest *req)
}
nvme_attach_ns(ctrl, ns);
- nvme_select_iocs_ns(ctrl, ns);
-
break;
case NVME_NS_ATTACHMENT_DETACH:
@@ -5720,26 +5718,12 @@ static uint16_t nvme_ns_attachment(NvmeCtrl *n,
NvmeRequest *req)
return NVME_NS_NOT_ATTACHED | NVME_DNR;
}
- ctrl->namespaces[nsid] = NULL;
- ns->attached--;
-
- nvme_update_dmrsl(ctrl);
-
+ nvme_detach_ns(ctrl, ns);
break;
default:
return NVME_INVALID_FIELD | NVME_DNR;
}
-
- /*
- * Add namespace id to the changed namespace id list for event clearing
- * via Get Log Page command.
- */
- if (!test_and_set_bit(nsid, ctrl->changed_nsids)) {
- nvme_enqueue_event(ctrl, NVME_AER_TYPE_NOTICE,
- NVME_AER_INFO_NOTICE_NS_ATTR_CHANGED,
- NVME_LOG_CHANGED_NSLIST);
- }
}
return NVME_SUCCESS;
@@ -7564,6 +7548,34 @@ void nvme_attach_ns(NvmeCtrl *n, NvmeNamespace *ns)
n->dmrsl = MIN_NON_ZERO(n->dmrsl,
BDRV_REQUEST_MAX_BYTES / nvme_l2b(ns, 1));
+ if (NVME_CC_EN(n->bar.cc)) {
+ /* Ctrl is live */
+ nvme_select_iocs_ns(n, ns);
+ if (!test_and_set_bit(nsid, n->changed_nsids)) {
+ nvme_enqueue_event(n, NVME_AER_TYPE_NOTICE,
+ NVME_AER_INFO_NOTICE_NS_ATTR_CHANGED,
+ NVME_LOG_CHANGED_NSLIST);
+ }
+ }
+}
+
+void nvme_detach_ns(NvmeCtrl *n, NvmeNamespace *ns)
+{
+ uint32_t nsid = ns->params.nsid;
+
+ if (ns->attached) {
+ n->namespaces[nsid - 1] = NULL;
+ ns->attached--;
+ }
+ nvme_update_dmrsl(n);
+ if (NVME_CC_EN(n->bar.cc)) {
+ /* Ctrl is live */
+ if (!test_and_set_bit(nsid, n->changed_nsids)) {
+ nvme_enqueue_event(n, NVME_AER_TYPE_NOTICE,
+ NVME_AER_INFO_NOTICE_NS_ATTR_CHANGED,
+ NVME_LOG_CHANGED_NSLIST);
+ }
+ }
}
static void nvme_realize(PCIDevice *pci_dev, Error **errp)
@@ -7600,6 +7612,7 @@ static void nvme_realize(PCIDevice *pci_dev, Error **errp)
return;
}
nvme_init_ctrl(n, pci_dev);
+ qbus_set_bus_hotplug_handler(BUS(&n->bus));
/* setup a namespace if the controller drive property was given */
if (n->namespace.blkconf.blk) {
@@ -7817,10 +7830,22 @@ static const TypeInfo nvme_info = {
},
};
+static void nvme_bus_class_init(ObjectClass *klass, void *data)
+{
+ HotplugHandlerClass *hc = HOTPLUG_HANDLER_CLASS(klass);
+
+ hc->unplug = qdev_simple_device_unplug_cb;
+}
+
static const TypeInfo nvme_bus_info = {
.name = TYPE_NVME_BUS,
.parent = TYPE_BUS,
.instance_size = sizeof(NvmeBus),
+ .class_init = nvme_bus_class_init,
+ .interfaces = (InterfaceInfo[]) {
+ { TYPE_HOTPLUG_HANDLER },
+ { }
+ }
};
static void nvme_register_types(void)
diff --git a/hw/nvme/ns.c b/hw/nvme/ns.c
index 62a1f97be0..69ba08b0e7 100644
--- a/hw/nvme/ns.c
+++ b/hw/nvme/ns.c
@@ -530,10 +530,31 @@ void nvme_ns_cleanup(NvmeNamespace *ns)
static void nvme_ns_unrealize(DeviceState *dev)
{
NvmeNamespace *ns = NVME_NS(dev);
+ BusState *s = qdev_get_parent_bus(dev);
+ NvmeCtrl *n = NVME(s->parent);
+ NvmeSubsystem *subsys = n->subsys;
+ uint32_t nsid = ns->params.nsid;
+ int i;
nvme_ns_drain(ns);
nvme_ns_shutdown(ns);
nvme_ns_cleanup(ns);
+
+ if (subsys) {
+ subsys->namespaces[nsid] = NULL;
+
+ if (ns->params.shared) {
+ for (i = 0; i < ARRAY_SIZE(subsys->ctrls); i++) {
+ NvmeCtrl *ctrl = subsys->ctrls[i];
+
+ if (ctrl) {
+ nvme_detach_ns(ctrl, ns);
+ }
+ }
+ return;
+ }
+ }
+ nvme_detach_ns(n, ns);
}
static void nvme_ns_realize(DeviceState *dev, Error **errp)
diff --git a/hw/nvme/nvme.h b/hw/nvme/nvme.h
index 79f5c281c2..7d9fc2ab28 100644
--- a/hw/nvme/nvme.h
+++ b/hw/nvme/nvme.h
@@ -576,6 +576,7 @@ static inline NvmeSecCtrlEntry
*nvme_sctrl_for_cntlid(NvmeCtrl *n,
}
void nvme_attach_ns(NvmeCtrl *n, NvmeNamespace *ns);
+void nvme_detach_ns(NvmeCtrl *n, NvmeNamespace *ns);
uint16_t nvme_bounce_data(NvmeCtrl *n, void *ptr, uint32_t len,
NvmeTxDirection dir, NvmeRequest *req);
uint16_t nvme_bounce_mdata(NvmeCtrl *n, void *ptr, uint32_t len,
