* [PATCH rfc 0/2] Support read-only subsystem critical warning @ 2017-12-04 9:24 Sagi Grimberg 2017-12-04 9:24 ` [PATCH rfc 1/2] nvme: support smart read-only critical warning event Sagi Grimberg 2017-12-04 9:24 ` [PATCH rfc 2/2] nvmet: expose option to emulate a nvm subsystem in read-only mode Sagi Grimberg 0 siblings, 2 replies; 14+ messages in thread From: Sagi Grimberg @ 2017-12-04 9:24 UTC (permalink / raw) In case a subsystem enters a read-only mode, the host shall issue a smart log page request and check the read-only bit in the critical warning section. In case its set, it shall set all the controller namespaces in read-only mode (or set the namespaces in read-write in case its cleared). The nvmet implements emulation of this to exercise the host (might also be useful for other use-cases as well though). Add a configfs knob to trigger read-only subsystem critical event. Also when the host connects to a read-only subsystem, and AER is queued to notify the host on the critical warning. Note that the spec does not refer to the scenario of fabrics connecting to an already read-only subsystem so the behavior I implemented is what I thought was most suitable. Also, the spec does not define (at least from what I found) what happens if a subsystem moves out from read-only state, so again, the behavior I implemented is what I found to be suitable. Sagi Grimberg (2): nvme: support smart read-only critical warning event nvmet: expose option to emulate a nvm subsystem in read-only mode drivers/nvme/host/core.c | 51 +++++++++++++++++++++++++++++++++++++++ drivers/nvme/host/multipath.c | 4 +++ drivers/nvme/host/nvme.h | 2 ++ drivers/nvme/target/admin-cmd.c | 3 +++ drivers/nvme/target/configfs.c | 34 ++++++++++++++++++++++++++ drivers/nvme/target/core.c | 2 +- drivers/nvme/target/fabrics-cmd.c | 3 +++ drivers/nvme/target/io-cmd.c | 8 +++++- drivers/nvme/target/nvmet.h | 3 +++ 9 files changed, 108 insertions(+), 2 deletions(-) -- 2.14.1 ^ permalink raw reply [flat|nested] 14+ messages in thread
* [PATCH rfc 1/2] nvme: support smart read-only critical warning event 2017-12-04 9:24 [PATCH rfc 0/2] Support read-only subsystem critical warning Sagi Grimberg @ 2017-12-04 9:24 ` Sagi Grimberg 2017-12-04 20:07 ` Christoph Hellwig 2017-12-04 9:24 ` [PATCH rfc 2/2] nvmet: expose option to emulate a nvm subsystem in read-only mode Sagi Grimberg 1 sibling, 1 reply; 14+ messages in thread From: Sagi Grimberg @ 2017-12-04 9:24 UTC (permalink / raw) In case a controller sent us a critical warning AER, we should query the smart information and in case the media was placed in read only mode, we need to set all the controller namespaces (and the namespaces heads) in read-only (or set to rw in case its not set). Given that in fabrics we may also connect to a read-only subsystem, we add the disk as read-only to start with. Once the subsystem will be in rw mode, we restore the disk rw permissions. Signed-off-by: Sagi Grimberg <sagi at grimberg.me> --- drivers/nvme/host/core.c | 51 +++++++++++++++++++++++++++++++++++++++++++ drivers/nvme/host/multipath.c | 4 ++++ drivers/nvme/host/nvme.h | 2 ++ 3 files changed, 57 insertions(+) diff --git a/drivers/nvme/host/core.c b/drivers/nvme/host/core.c index 25e3168cec44..eaae1b4f6a4e 100644 --- a/drivers/nvme/host/core.c +++ b/drivers/nvme/host/core.c @@ -2949,6 +2949,9 @@ static void nvme_alloc_ns(struct nvme_ctrl *ctrl, unsigned nsid) kfree(id); + if (ctrl->subsys->read_only) + set_disk_ro(ns->disk, true); + device_add_disk(ctrl->device, ns->disk); if (sysfs_create_group(&disk_to_dev(ns->disk)->kobj, &nvme_ns_id_attr_group)) @@ -3192,6 +3195,49 @@ static void nvme_get_fw_slot_info(struct nvme_ctrl *ctrl) kfree(log); } +static void nvme_get_smart_log(struct nvme_ctrl *ctrl) +{ + struct nvme_smart_log *log; + struct nvme_ns *ns; + + log = kmalloc(sizeof(*log), GFP_KERNEL); + if (!log) + return; + + if (nvme_get_log(ctrl, NVME_LOG_SMART, log, sizeof(*log))) { + dev_warn(ctrl->device, "Get SMART log error\n"); + goto out; + } + + if (log->critical_warning & NVME_SMART_CRIT_MEDIA) { + dev_warn(ctrl->device, + "critical warning: media placed in read-only\n"); + ctrl->subsys->read_only = true; + } else { + dev_info(ctrl->device, + "media placed in read-write mode\n"); + ctrl->subsys->read_only = false; + } + + mutex_lock(&ctrl->namespaces_mutex); + list_for_each_entry(ns, &ctrl->namespaces, list) { + set_disk_ro(ns->disk, ctrl->subsys->read_only); + if (ns->head->disk) + set_disk_ro(ns->head->disk, ctrl->subsys->read_only); + } + mutex_unlock(&ctrl->namespaces_mutex); +out: + kfree(log); +} + +static void nvme_smart_work(struct work_struct *work) +{ + struct nvme_ctrl *ctrl = + container_of(work, struct nvme_ctrl, smart_work); + + nvme_get_smart_log(ctrl); +} + static void nvme_fw_act_work(struct work_struct *work) { struct nvme_ctrl *ctrl = container_of(work, @@ -3251,6 +3297,9 @@ void nvme_complete_async_event(struct nvme_ctrl *ctrl, __le16 status, case NVME_AER_NOTICE_FW_ACT_STARTING: queue_work(nvme_wq, &ctrl->fw_act_work); break; + case NVME_AER_SMART: + queue_work(nvme_wq, &ctrl->smart_work); + break; default: dev_warn(ctrl->device, "async event result %08x\n", result); } @@ -3263,6 +3312,7 @@ void nvme_stop_ctrl(struct nvme_ctrl *ctrl) nvme_stop_keep_alive(ctrl); flush_work(&ctrl->async_event_work); flush_work(&ctrl->scan_work); + flush_work(&ctrl->smart_work); cancel_work_sync(&ctrl->fw_act_work); } EXPORT_SYMBOL_GPL(nvme_stop_ctrl); @@ -3328,6 +3378,7 @@ int nvme_init_ctrl(struct nvme_ctrl *ctrl, struct device *dev, INIT_WORK(&ctrl->scan_work, nvme_scan_work); INIT_WORK(&ctrl->async_event_work, nvme_async_event_work); INIT_WORK(&ctrl->fw_act_work, nvme_fw_act_work); + INIT_WORK(&ctrl->smart_work, nvme_smart_work); INIT_WORK(&ctrl->delete_work, nvme_delete_ctrl_work); ret = ida_simple_get(&nvme_instance_ida, 0, 0, GFP_KERNEL); diff --git a/drivers/nvme/host/multipath.c b/drivers/nvme/host/multipath.c index 1218a9fca846..b833ba3c78cb 100644 --- a/drivers/nvme/host/multipath.c +++ b/drivers/nvme/host/multipath.c @@ -238,6 +238,10 @@ void nvme_mpath_add_disk(struct nvme_ns_head *head) { if (!head->disk) return; + + if (head->subsys->read_only) + set_disk_ro(head->disk, true); + device_add_disk(&head->subsys->dev, head->disk); if (sysfs_create_group(&disk_to_dev(head->disk)->kobj, &nvme_ns_id_attr_group)) diff --git a/drivers/nvme/host/nvme.h b/drivers/nvme/host/nvme.h index 4361b71891e9..caa8f7130ec6 100644 --- a/drivers/nvme/host/nvme.h +++ b/drivers/nvme/host/nvme.h @@ -181,6 +181,7 @@ struct nvme_ctrl { struct nvme_id_power_state psd[32]; struct nvme_effects_log *effects; struct work_struct scan_work; + struct work_struct smart_work; struct work_struct async_event_work; struct delayed_work ka_work; struct work_struct fw_act_work; @@ -226,6 +227,7 @@ struct nvme_subsystem { u8 cmic; u16 vendor_id; struct ida ns_ida; + bool read_only; }; /* -- 2.14.1 ^ permalink raw reply related [flat|nested] 14+ messages in thread
* [PATCH rfc 1/2] nvme: support smart read-only critical warning event 2017-12-04 9:24 ` [PATCH rfc 1/2] nvme: support smart read-only critical warning event Sagi Grimberg @ 2017-12-04 20:07 ` Christoph Hellwig 2017-12-05 7:28 ` Sagi Grimberg 0 siblings, 1 reply; 14+ messages in thread From: Christoph Hellwig @ 2017-12-04 20:07 UTC (permalink / raw) On Mon, Dec 04, 2017@11:24:34AM +0200, Sagi Grimberg wrote: > In case a controller sent us a critical warning AER, > we should query the smart information and in case the > media was placed in read only mode, we need to set > all the controller namespaces (and the namespaces heads) > in read-only (or set to rw in case its not set). > > Given that in fabrics we may also connect to a read-only > subsystem, we add the disk as read-only to start with. > Once the subsystem will be in rw mode, we restore > the disk rw permissions. We are sending the smart log notification on to userspace, so with this we now have two parties that are trying to clear the log page.. ^ permalink raw reply [flat|nested] 14+ messages in thread
* [PATCH rfc 1/2] nvme: support smart read-only critical warning event 2017-12-04 20:07 ` Christoph Hellwig @ 2017-12-05 7:28 ` Sagi Grimberg 2017-12-05 7:57 ` Johannes Thumshirn 2017-12-06 22:47 ` Christoph Hellwig 0 siblings, 2 replies; 14+ messages in thread From: Sagi Grimberg @ 2017-12-05 7:28 UTC (permalink / raw) >> In case a controller sent us a critical warning AER, >> we should query the smart information and in case the >> media was placed in read only mode, we need to set >> all the controller namespaces (and the namespaces heads) >> in read-only (or set to rw in case its not set). >> >> Given that in fabrics we may also connect to a read-only >> subsystem, we add the disk as read-only to start with. >> Once the subsystem will be in rw mode, we restore >> the disk rw permissions. > > We are sending the smart log notification on to userspace, so with this > we now have two parties that are trying to clear the log page.. Not at the moment, but soon we will send smart logs to userspace, yes. I guess we have two choices for this: 1. let userspace handle it and expose a knob to modify all controller namespaces to r/o. 2. handle the event in the kernel, issue the log page with retain bit on and only then notify userspace. I personally prefer (2), what do others think? ^ permalink raw reply [flat|nested] 14+ messages in thread
* [PATCH rfc 1/2] nvme: support smart read-only critical warning event 2017-12-05 7:28 ` Sagi Grimberg @ 2017-12-05 7:57 ` Johannes Thumshirn 2017-12-06 22:47 ` Christoph Hellwig 1 sibling, 0 replies; 14+ messages in thread From: Johannes Thumshirn @ 2017-12-05 7:57 UTC (permalink / raw) Sagi Grimberg <sagi at grimberg.me> writes: > 2. handle the event in the kernel, issue the log page with retain bit on > and only then notify userspace. > > I personally prefer (2), what do others think? Sorry for chiming in so late, but I agree with Sagi. Byte, Johannes -- Johannes Thumshirn Storage jthumshirn at suse.de +49 911 74053 689 SUSE LINUX GmbH, Maxfeldstr. 5, 90409 N?rnberg GF: Felix Imend?rffer, Jane Smithard, Graham Norton HRB 21284 (AG N?rnberg) Key fingerprint = EC38 9CAB C2C4 F25D 8600 D0D0 0393 969D 2D76 0850 ^ permalink raw reply [flat|nested] 14+ messages in thread
* [PATCH rfc 1/2] nvme: support smart read-only critical warning event 2017-12-05 7:28 ` Sagi Grimberg 2017-12-05 7:57 ` Johannes Thumshirn @ 2017-12-06 22:47 ` Christoph Hellwig 2017-12-07 6:45 ` Sagi Grimberg 1 sibling, 1 reply; 14+ messages in thread From: Christoph Hellwig @ 2017-12-06 22:47 UTC (permalink / raw) On Tue, Dec 05, 2017@09:28:53AM +0200, Sagi Grimberg wrote: > 2. handle the event in the kernel, issue the log page with retain bit on > and only then notify userspace. The retain bit is a new optional feature in nvme 1.3. I fear we won't see all that many controllers respecting it. ^ permalink raw reply [flat|nested] 14+ messages in thread
* [PATCH rfc 1/2] nvme: support smart read-only critical warning event 2017-12-06 22:47 ` Christoph Hellwig @ 2017-12-07 6:45 ` Sagi Grimberg 0 siblings, 0 replies; 14+ messages in thread From: Sagi Grimberg @ 2017-12-07 6:45 UTC (permalink / raw) >> 2. handle the event in the kernel, issue the log page with retain bit on >> and only then notify userspace. > > The retain bit is a new optional feature in nvme 1.3. I fear we > won't see all that many controllers respecting it. Well, for those devices user-space will see a cleared log page... Unless you prefer option 1? Or have a better idea? ^ permalink raw reply [flat|nested] 14+ messages in thread
* [PATCH rfc 2/2] nvmet: expose option to emulate a nvm subsystem in read-only mode 2017-12-04 9:24 [PATCH rfc 0/2] Support read-only subsystem critical warning Sagi Grimberg 2017-12-04 9:24 ` [PATCH rfc 1/2] nvme: support smart read-only critical warning event Sagi Grimberg @ 2017-12-04 9:24 ` Sagi Grimberg 2017-12-04 16:19 ` Christoph Hellwig 1 sibling, 1 reply; 14+ messages in thread From: Sagi Grimberg @ 2017-12-04 9:24 UTC (permalink / raw) Useful for exercising the host. Also, if a host connects to a subsystem which is read-only to begin with, queue an AER to let it know about it. In read-only state, we fail writes/dsms/write-zeros. Signed-off-by: Sagi Grimberg <sagi at grimberg.me> --- drivers/nvme/target/admin-cmd.c | 3 +++ drivers/nvme/target/configfs.c | 34 ++++++++++++++++++++++++++++++++++ drivers/nvme/target/core.c | 2 +- drivers/nvme/target/fabrics-cmd.c | 3 +++ drivers/nvme/target/io-cmd.c | 8 +++++++- drivers/nvme/target/nvmet.h | 3 +++ 6 files changed, 51 insertions(+), 2 deletions(-) diff --git a/drivers/nvme/target/admin-cmd.c b/drivers/nvme/target/admin-cmd.c index 6b921c253618..aab646f44a66 100644 --- a/drivers/nvme/target/admin-cmd.c +++ b/drivers/nvme/target/admin-cmd.c @@ -69,6 +69,9 @@ static u16 nvmet_get_smart_log_all(struct nvmet_req *req, ctrl = req->sq->ctrl; + if(req->sq->ctrl->subsys->read_only) + slog->critical_warning |= NVME_SMART_CRIT_MEDIA; + rcu_read_lock(); list_for_each_entry_rcu(ns, &ctrl->subsys->namespaces, dev_link) { host_reads += part_stat_read(ns->bdev->bd_part, ios[READ]); diff --git a/drivers/nvme/target/configfs.c b/drivers/nvme/target/configfs.c index 8f378030c8e4..c966228439f6 100644 --- a/drivers/nvme/target/configfs.c +++ b/drivers/nvme/target/configfs.c @@ -711,6 +711,39 @@ static ssize_t nvmet_subsys_attr_allow_any_host_store(struct config_item *item, CONFIGFS_ATTR(nvmet_subsys_, attr_allow_any_host); +static ssize_t nvmet_subsys_attr_ro_show(struct config_item *item, + char *page) +{ + return snprintf(page, PAGE_SIZE, "%d\n", + to_subsys(item)->read_only); +} + +static ssize_t nvmet_subsys_attr_ro_store(struct config_item *item, + const char *page, size_t count) +{ + struct nvmet_subsys *subsys = to_subsys(item); + bool subsys_ro = subsys->read_only; + bool ro; + + if (strtobool(page, &ro)) + return -EINVAL; + + subsys->read_only = ro; + if ((!subsys_ro && ro) || (subsys_ro && !ro)) { + struct nvmet_ctrl *ctrl; + + /* read only state change, send a SMART AER */ + mutex_lock(&subsys->lock); + list_for_each_entry(ctrl, &subsys->ctrls, subsys_entry) + nvmet_add_async_event(ctrl, NVME_AER_SMART, 0, 0); + mutex_unlock(&subsys->lock); + } + + return count; +} + +CONFIGFS_ATTR(nvmet_subsys_, attr_ro); + static ssize_t nvmet_subsys_attr_version_show(struct config_item *item, char *page) { @@ -770,6 +803,7 @@ CONFIGFS_ATTR(nvmet_subsys_, attr_serial); static struct configfs_attribute *nvmet_subsys_attrs[] = { &nvmet_subsys_attr_attr_allow_any_host, + &nvmet_subsys_attr_attr_ro, &nvmet_subsys_attr_attr_version, &nvmet_subsys_attr_attr_serial, NULL, diff --git a/drivers/nvme/target/core.c b/drivers/nvme/target/core.c index 841a37dfb987..5b4d30db9776 100644 --- a/drivers/nvme/target/core.c +++ b/drivers/nvme/target/core.c @@ -117,7 +117,7 @@ static void nvmet_async_event_work(struct work_struct *work) } } -static void nvmet_add_async_event(struct nvmet_ctrl *ctrl, u8 event_type, +void nvmet_add_async_event(struct nvmet_ctrl *ctrl, u8 event_type, u8 event_info, u8 log_page) { struct nvmet_async_event *aen; diff --git a/drivers/nvme/target/fabrics-cmd.c b/drivers/nvme/target/fabrics-cmd.c index 62315cc4b5b0..fc9ef44d2b49 100644 --- a/drivers/nvme/target/fabrics-cmd.c +++ b/drivers/nvme/target/fabrics-cmd.c @@ -177,6 +177,9 @@ static void nvmet_execute_admin_connect(struct nvmet_req *req) goto out; } + if(req->sq->ctrl->subsys->read_only) + nvmet_add_async_event(req->sq->ctrl, NVME_AER_SMART, 0, 0); + pr_info("creating controller %d for subsystem %s for NQN %s.\n", ctrl->cntlid, ctrl->subsys->subsysnqn, ctrl->hostnqn); req->rsp->result.u16 = cpu_to_le16(ctrl->cntlid); diff --git a/drivers/nvme/target/io-cmd.c b/drivers/nvme/target/io-cmd.c index 0a4372a016f2..6f121d2f2ea3 100644 --- a/drivers/nvme/target/io-cmd.c +++ b/drivers/nvme/target/io-cmd.c @@ -202,8 +202,10 @@ u16 nvmet_parse_io_cmd(struct nvmet_req *req) return NVME_SC_INVALID_NS | NVME_SC_DNR; switch (cmd->common.opcode) { - case nvme_cmd_read: case nvme_cmd_write: + if(unlikely(req->sq->ctrl->subsys->read_only)) + return NVME_SC_READ_ONLY | NVME_SC_DNR; + case nvme_cmd_read: req->execute = nvmet_execute_rw; req->data_len = nvmet_rw_len(req); return 0; @@ -212,11 +214,15 @@ u16 nvmet_parse_io_cmd(struct nvmet_req *req) req->data_len = 0; return 0; case nvme_cmd_dsm: + if(unlikely(req->sq->ctrl->subsys->read_only)) + return NVME_SC_READ_ONLY | NVME_SC_DNR; req->execute = nvmet_execute_dsm; req->data_len = (le32_to_cpu(cmd->dsm.nr) + 1) * sizeof(struct nvme_dsm_range); return 0; case nvme_cmd_write_zeroes: + if(unlikely(req->sq->ctrl->subsys->read_only)) + return NVME_SC_READ_ONLY | NVME_SC_DNR; req->execute = nvmet_execute_write_zeroes; return 0; default: diff --git a/drivers/nvme/target/nvmet.h b/drivers/nvme/target/nvmet.h index d69d948c8f36..a9fb763020b6 100644 --- a/drivers/nvme/target/nvmet.h +++ b/drivers/nvme/target/nvmet.h @@ -144,6 +144,7 @@ struct nvmet_ctrl { struct nvmet_subsys { enum nvme_subsys_type type; + bool read_only; struct mutex lock; struct kref ref; @@ -287,6 +288,8 @@ void nvmet_sq_destroy(struct nvmet_sq *sq); int nvmet_sq_init(struct nvmet_sq *sq); void nvmet_ctrl_fatal_error(struct nvmet_ctrl *ctrl); +void nvmet_add_async_event(struct nvmet_ctrl *ctrl, u8 event_type, + u8 event_info, u8 log_page); void nvmet_update_cc(struct nvmet_ctrl *ctrl, u32 new); u16 nvmet_alloc_ctrl(const char *subsysnqn, const char *hostnqn, -- 2.14.1 ^ permalink raw reply related [flat|nested] 14+ messages in thread
* [PATCH rfc 2/2] nvmet: expose option to emulate a nvm subsystem in read-only mode 2017-12-04 9:24 ` [PATCH rfc 2/2] nvmet: expose option to emulate a nvm subsystem in read-only mode Sagi Grimberg @ 2017-12-04 16:19 ` Christoph Hellwig 2017-12-04 18:11 ` Sagi Grimberg 0 siblings, 1 reply; 14+ messages in thread From: Christoph Hellwig @ 2017-12-04 16:19 UTC (permalink / raw) I don't think that is what the SMART log is inteded for. Please review the write protected namespace TP in the working group instead and make sure it passes soon so that we can use it. ^ permalink raw reply [flat|nested] 14+ messages in thread
* [PATCH rfc 2/2] nvmet: expose option to emulate a nvm subsystem in read-only mode 2017-12-04 16:19 ` Christoph Hellwig @ 2017-12-04 18:11 ` Sagi Grimberg 2017-12-04 20:06 ` Christoph Hellwig 0 siblings, 1 reply; 14+ messages in thread From: Sagi Grimberg @ 2017-12-04 18:11 UTC (permalink / raw) Hey Christoph, > I don't think that is what the SMART log is inteded for. Can you please elaborate what you mean? are you talking about this patch in particular (simply an emulation)? or support for this altogether? A subsystem can, at any point, be placed in read-only mode. In that case it needs to send SMART AER to indicate health status change. I think that if the media has been placed in read-only mode, we need to log it, and set all the subsystem namespaces in read-only mode. Anything different you think we need to do here? > Please review the write protected namespace TP in the working group instead > and make sure it passes soon so that we can use it. I did look at write protect, this is not the use-case here. write-protect is a per-namespace attribute which is controlled by the host, this addresses NVMe subsystem critical warning scenario. ^ permalink raw reply [flat|nested] 14+ messages in thread
* [PATCH rfc 2/2] nvmet: expose option to emulate a nvm subsystem in read-only mode 2017-12-04 18:11 ` Sagi Grimberg @ 2017-12-04 20:06 ` Christoph Hellwig 2017-12-05 7:22 ` Sagi Grimberg 0 siblings, 1 reply; 14+ messages in thread From: Christoph Hellwig @ 2017-12-04 20:06 UTC (permalink / raw) On Mon, Dec 04, 2017@08:11:14PM +0200, Sagi Grimberg wrote: > Can you please elaborate what you mean? are you talking about this > patch in particular (simply an emulation)? or support for this > altogether? A subsystem can, at any point, be placed in read-only mode. > In that case it needs to send SMART AER to indicate health status > change. I think that if the media has been placed in read-only mode, we > need to log it, and set all the subsystem namespaces in read-only mode. > > Anything different you think we need to do here? Yes, but for that we should trigger it off gettind a EROFS warning from the block device, not through sysfs. Having the explicit sysfs no, and the code to send the AER on connect suggests to me that this was intended to be used to emulate a r/o setting, but apparently I was wrong. ^ permalink raw reply [flat|nested] 14+ messages in thread
* [PATCH rfc 2/2] nvmet: expose option to emulate a nvm subsystem in read-only mode 2017-12-04 20:06 ` Christoph Hellwig @ 2017-12-05 7:22 ` Sagi Grimberg 2017-12-06 22:46 ` Christoph Hellwig 0 siblings, 1 reply; 14+ messages in thread From: Sagi Grimberg @ 2017-12-05 7:22 UTC (permalink / raw) >> Can you please elaborate what you mean? are you talking about this >> patch in particular (simply an emulation)? or support for this >> altogether? A subsystem can, at any point, be placed in read-only mode. >> In that case it needs to send SMART AER to indicate health status >> change. I think that if the media has been placed in read-only mode, we >> need to log it, and set all the subsystem namespaces in read-only mode. >> >> Anything different you think we need to do here? > > Yes, but for that we should trigger it off gettind a EROFS warning > from the block device, not through sysfs. Just to be clear, that is exactly what I intended to do. fs/block_dev.c checks bdev_read_only() in blkdev_write_iter and also there is a patchset to enforce this for several places in the block layer from Ilya Dryomov ("block: enforce ioctl(BLKROSET) and set_disk_ro()") I wander if we need this also in generic_write_checks()? > Having the explicit sysfs > no, and the code to send the AER on connect suggests to me that > this was intended to be used to emulate a r/o setting, but apparently > I was wrong. I'm not sure I understand your point here, can you explain what you mean? nvmet sends a smart AER in case a host connected to a subsystem in r/o state because there no other way it will know that (other than failing writes when they come). Its just an attempt to notify the host right away instead of waiting for it to issue writes and fail later. ^ permalink raw reply [flat|nested] 14+ messages in thread
* [PATCH rfc 2/2] nvmet: expose option to emulate a nvm subsystem in read-only mode 2017-12-05 7:22 ` Sagi Grimberg @ 2017-12-06 22:46 ` Christoph Hellwig 2017-12-07 6:41 ` Sagi Grimberg 0 siblings, 1 reply; 14+ messages in thread From: Christoph Hellwig @ 2017-12-06 22:46 UTC (permalink / raw) On Tue, Dec 05, 2017@09:22:05AM +0200, Sagi Grimberg wrote: > Just to be clear, that is exactly what I intended to do. My point is that we should not have a configfs nob to turn on the r/o nob, but do it based on an error from the lower block device. ^ permalink raw reply [flat|nested] 14+ messages in thread
* [PATCH rfc 2/2] nvmet: expose option to emulate a nvm subsystem in read-only mode 2017-12-06 22:46 ` Christoph Hellwig @ 2017-12-07 6:41 ` Sagi Grimberg 0 siblings, 0 replies; 14+ messages in thread From: Sagi Grimberg @ 2017-12-07 6:41 UTC (permalink / raw) >> Just to be clear, that is exactly what I intended to do. > > My point is that we should not have a configfs nob to turn > on the r/o nob, but do it based on an error from the lower > block device. What do you suggest we do when the subsystem exports two block devices of two different subsystems where only one gets into r/o state? ^ permalink raw reply [flat|nested] 14+ messages in thread
end of thread, other threads:[~2017-12-07 6:45 UTC | newest] Thread overview: 14+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2017-12-04 9:24 [PATCH rfc 0/2] Support read-only subsystem critical warning Sagi Grimberg 2017-12-04 9:24 ` [PATCH rfc 1/2] nvme: support smart read-only critical warning event Sagi Grimberg 2017-12-04 20:07 ` Christoph Hellwig 2017-12-05 7:28 ` Sagi Grimberg 2017-12-05 7:57 ` Johannes Thumshirn 2017-12-06 22:47 ` Christoph Hellwig 2017-12-07 6:45 ` Sagi Grimberg 2017-12-04 9:24 ` [PATCH rfc 2/2] nvmet: expose option to emulate a nvm subsystem in read-only mode Sagi Grimberg 2017-12-04 16:19 ` Christoph Hellwig 2017-12-04 18:11 ` Sagi Grimberg 2017-12-04 20:06 ` Christoph Hellwig 2017-12-05 7:22 ` Sagi Grimberg 2017-12-06 22:46 ` Christoph Hellwig 2017-12-07 6:41 ` Sagi Grimberg
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.