[libvirt] [PATCH v4] qemu: Validate address type when attaching a disk device.
Bug fix for: https://bugzilla.redhat.com/show_bug.cgi?id=1257844 Attach-device can hotplug a virtio disk device with any address type now, it need to validate the address type before the attachment. Attaching a disk device with --config option need to check address type. Otherwise, the domain cloudn't be started normally. This patch add a basic check for address type. Detaching a disk device with --config also need to check disk options, add qemuCheckDiskConfig() method in qemuDomainDetachDeviceConfig(). Add virDomainDeviceAddressTypeIsValid method in domain_conf to check disk address type. Address type should match with disk driver, report error messages if any mismatch. Signed-off-by: Ruifeng Bian <rbian@redhat.com> --- src/conf/domain_conf.c | 32 ++++++++++++++++++++++++++++++++ src/conf/domain_conf.h | 1 + src/libvirt_private.syms | 1 + src/qemu/qemu_command.c | 7 +++++++ src/qemu/qemu_driver.c | 2 ++ src/qemu/qemu_hotplug.c | 22 ++++++++++++++++++++++ 6 files changed, 65 insertions(+) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 6df1618..f86760b 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -3200,6 +3200,38 @@ int virDomainDeviceAddressIsValid(virDomainDeviceInfoPtr info, return 0; } +int virDomainDeviceAddressTypeIsValid(virDomainDiskDefPtr disk) +{ + if (disk->info.type) { + switch (disk->bus) { + case VIR_DOMAIN_DISK_BUS_IDE: + case VIR_DOMAIN_DISK_BUS_SCSI: + case VIR_DOMAIN_DISK_BUS_SATA: + case VIR_DOMAIN_DISK_BUS_FDC: + if (disk->info.type != VIR_DOMAIN_DEVICE_ADDRESS_TYPE_DRIVE) + return 1; + break; + case VIR_DOMAIN_DISK_BUS_USB: + if (disk->info.type != VIR_DOMAIN_DEVICE_ADDRESS_TYPE_USB) + return 1; + break; + case VIR_DOMAIN_DISK_BUS_VIRTIO: + if (disk->info.type != VIR_DOMAIN_DEVICE_ADDRESS_TYPE_CCW && + disk->info.type != VIR_DOMAIN_DEVICE_ADDRESS_TYPE_VIRTIO_S390 && + disk->info.type != VIR_DOMAIN_DEVICE_ADDRESS_TYPE_VIRTIO_MMIO && + disk->info.type != VIR_DOMAIN_DEVICE_ADDRESS_TYPE_PCI) + return 1; + break; + case VIR_DOMAIN_DISK_BUS_XEN: + case VIR_DOMAIN_DISK_BUS_UML: + case VIR_DOMAIN_DISK_BUS_SD: + /* no address type currently, return false if address provided */ + return 1; + } + } + return 0; +} + virDomainDeviceInfoPtr virDomainDeviceGetInfo(virDomainDeviceDefPtr device) { diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h index f043554..337fe51 100644 --- a/src/conf/domain_conf.h +++ b/src/conf/domain_conf.h @@ -2543,6 +2543,7 @@ virDomainDeviceDefPtr virDomainDeviceDefCopy(virDomainDeviceDefPtr src, virDomainXMLOptionPtr xmlopt); int virDomainDeviceAddressIsValid(virDomainDeviceInfoPtr info, int type); +int virDomainDeviceAddressTypeIsValid(virDomainDiskDefPtr disk); virDomainDeviceInfoPtr virDomainDeviceGetInfo(virDomainDeviceDefPtr device); int virDomainDeviceInfoCopy(virDomainDeviceInfoPtr dst, virDomainDeviceInfoPtr src); diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index 5b3da83..6cd5b9e 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -229,6 +229,7 @@ virDomainDefPostParse; virDomainDefSetMemoryInitial; virDomainDeleteConfig; virDomainDeviceAddressIsValid; +virDomainDeviceAddressTypeIsValid; virDomainDeviceAddressTypeToString; virDomainDeviceDefCheckUnsupportedMemoryDevice; virDomainDeviceDefCopy; diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index 25f57f2..56ba08d 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -3606,6 +3606,13 @@ qemuCheckDiskConfig(virDomainDiskDefPtr disk) goto error; } } + + if (virDomainDeviceAddressTypeIsValid(disk)) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, + _("disk cannot have an address of type '%s'"), + virDomainDeviceAddressTypeToString(disk->info.type)); + goto error; + } return 0; error: return -1; diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index fcf86b6..26c1502 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -8208,6 +8208,8 @@ qemuDomainDetachDeviceConfig(virDomainDefPtr vmdef, switch ((virDomainDeviceType) dev->type) { case VIR_DOMAIN_DEVICE_DISK: disk = dev->data.disk; + if (qemuCheckDiskConfig(disk) < 0) + return -1; if (!(det_disk = virDomainDiskRemoveByName(vmdef, disk->dst))) { virReportError(VIR_ERR_INVALID_ARG, _("no target device %s"), disk->dst); diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c index e84a78d..6156243 100644 --- a/src/qemu/qemu_hotplug.c +++ b/src/qemu/qemu_hotplug.c @@ -336,6 +336,28 @@ qemuDomainAttachVirtioDiskDevice(virConnectPtr conn, if (!qemuCheckCCWS390AddressSupport(vm->def, disk->info, priv->qemuCaps, disk->dst)) goto cleanup; + + if (qemuDomainMachineIsS390CCW(vm->def) && + virQEMUCapsGet(priv->qemuCaps, QEMU_CAPS_VIRTIO_CCW)) { + if (!virDomainDeviceAddressIsValid(&disk->info, + VIR_DOMAIN_DEVICE_ADDRESS_TYPE_CCW)) { + virReportError(VIR_ERR_OPERATION_FAILED, "%s", + _("device cannot be attached without a valid CCW address")); + goto cleanup; + } + } else if (virQEMUCapsGet(priv->qemuCaps, QEMU_CAPS_VIRTIO_S390)) { + if (!virDomainDeviceAddressIsValid(&disk->info, + VIR_DOMAIN_DEVICE_ADDRESS_TYPE_VIRTIO_S390)) { + virReportError(VIR_ERR_OPERATION_FAILED, "%s", + _("device cannot be attached without a valid S390 address")); + goto cleanup; + } + } else if (!virDomainDeviceAddressIsValid(&disk->info, + VIR_DOMAIN_DEVICE_ADDRESS_TYPE_PCI)) { + virReportError(VIR_ERR_OPERATION_FAILED, "%s", + _("device cannot be attached without a valid PCI address")); + goto cleanup; + } } for (i = 0; i < vm->def->ndisks; i++) { -- 2.4.3
On 09/17/2015 03:35 AM, Ruifeng Bian wrote:
Bug fix for: https://bugzilla.redhat.com/show_bug.cgi?id=1257844
Attach-device can hotplug a virtio disk device with any address type now, it need to validate the address type before the attachment.
Attaching a disk device with --config option need to check address type. Otherwise, the domain cloudn't be started normally. This patch add a basic check for address type.
Detaching a disk device with --config also need to check disk options, add qemuCheckDiskConfig() method in qemuDomainDetachDeviceConfig().
Add virDomainDeviceAddressTypeIsValid method in domain_conf to check disk address type. Address type should match with disk driver, report error messages if any mismatch.
Signed-off-by: Ruifeng Bian <rbian@redhat.com> --- src/conf/domain_conf.c | 32 ++++++++++++++++++++++++++++++++ src/conf/domain_conf.h | 1 + src/libvirt_private.syms | 1 + src/qemu/qemu_command.c | 7 +++++++ src/qemu/qemu_driver.c | 2 ++ src/qemu/qemu_hotplug.c | 22 ++++++++++++++++++++++ 6 files changed, 65 insertions(+)
Just so this isn't left "hanging"... Michal has generated an alternate method to address the issue, see: http://www.redhat.com/archives/libvir-list/2015-September/msg00909.html I think there could be "synergies" between the two and left some thoughts in the other patch which use some of this code. Some other comments below since they may be useful...
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 6df1618..f86760b 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -3200,6 +3200,38 @@ int virDomainDeviceAddressIsValid(virDomainDeviceInfoPtr info, return 0; }
+int virDomainDeviceAddressTypeIsValid(virDomainDiskDefPtr disk)
should be: bool virDomainDeviceAddressTypeIsValid(virDomainDiskDefPtr disk)
+{ + if (disk->info.type) { + switch (disk->bus) { + case VIR_DOMAIN_DISK_BUS_IDE: + case VIR_DOMAIN_DISK_BUS_SCSI: + case VIR_DOMAIN_DISK_BUS_SATA: + case VIR_DOMAIN_DISK_BUS_FDC: + if (disk->info.type != VIR_DOMAIN_DEVICE_ADDRESS_TYPE_DRIVE) + return 1;
return false;
+ break; + case VIR_DOMAIN_DISK_BUS_USB: + if (disk->info.type != VIR_DOMAIN_DEVICE_ADDRESS_TYPE_USB) + return 1;
return false;
+ break; + case VIR_DOMAIN_DISK_BUS_VIRTIO: + if (disk->info.type != VIR_DOMAIN_DEVICE_ADDRESS_TYPE_CCW && + disk->info.type != VIR_DOMAIN_DEVICE_ADDRESS_TYPE_VIRTIO_S390 && + disk->info.type != VIR_DOMAIN_DEVICE_ADDRESS_TYPE_VIRTIO_MMIO && + disk->info.type != VIR_DOMAIN_DEVICE_ADDRESS_TYPE_PCI) + return 1;
return false;
+ break; + case VIR_DOMAIN_DISK_BUS_XEN: + case VIR_DOMAIN_DISK_BUS_UML: + case VIR_DOMAIN_DISK_BUS_SD: + /* no address type currently, return false if address provided */ + return 1;
return false;
+ } + } + return 0;
return true;
+} + virDomainDeviceInfoPtr virDomainDeviceGetInfo(virDomainDeviceDefPtr device) { diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h index f043554..337fe51 100644 --- a/src/conf/domain_conf.h +++ b/src/conf/domain_conf.h @@ -2543,6 +2543,7 @@ virDomainDeviceDefPtr virDomainDeviceDefCopy(virDomainDeviceDefPtr src, virDomainXMLOptionPtr xmlopt); int virDomainDeviceAddressIsValid(virDomainDeviceInfoPtr info, int type); +int virDomainDeviceAddressTypeIsValid(virDomainDiskDefPtr disk);
s/int/bool, probably unnecessary though
virDomainDeviceInfoPtr virDomainDeviceGetInfo(virDomainDeviceDefPtr device); int virDomainDeviceInfoCopy(virDomainDeviceInfoPtr dst, virDomainDeviceInfoPtr src); diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index 5b3da83..6cd5b9e 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -229,6 +229,7 @@ virDomainDefPostParse; virDomainDefSetMemoryInitial; virDomainDeleteConfig; virDomainDeviceAddressIsValid; +virDomainDeviceAddressTypeIsValid;
probably unnecessary
virDomainDeviceAddressTypeToString; virDomainDeviceDefCheckUnsupportedMemoryDevice; virDomainDeviceDefCopy; diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index 25f57f2..56ba08d 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -3606,6 +3606,13 @@ qemuCheckDiskConfig(virDomainDiskDefPtr disk) goto error; } } + + if (virDomainDeviceAddressTypeIsValid(disk)) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, + _("disk cannot have an address of type '%s'"), + virDomainDeviceAddressTypeToString(disk->info.type));
how about disk target '%s' using bus '%s' cannot have an address of type '%s', where target would be disk->dst and the bus would be decoded via virDomainDiskBusTypeToString(disk->bus), results in error message such as: error: unsupported configuration: disk target 'vde' using bus 'virtio' cannot have an address of type 'drive'
+ goto error; + }
But I'm not convinced the check should be put it here.
return 0; error: return -1; diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index fcf86b6..26c1502 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -8208,6 +8208,8 @@ qemuDomainDetachDeviceConfig(virDomainDefPtr vmdef, switch ((virDomainDeviceType) dev->type) { case VIR_DOMAIN_DEVICE_DISK: disk = dev->data.disk; + if (qemuCheckDiskConfig(disk) < 0) + return -1;
Why are we checking at Detach? Doesn't that already fail? Or is this just to get the "new" error message?
if (!(det_disk = virDomainDiskRemoveByName(vmdef, disk->dst))) { virReportError(VIR_ERR_INVALID_ARG, _("no target device %s"), disk->dst); diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c index e84a78d..6156243 100644 --- a/src/qemu/qemu_hotplug.c +++ b/src/qemu/qemu_hotplug.c @@ -336,6 +336,28 @@ qemuDomainAttachVirtioDiskDevice(virConnectPtr conn, if (!qemuCheckCCWS390AddressSupport(vm->def, disk->info, priv->qemuCaps, disk->dst)) goto cleanup; + + if (qemuDomainMachineIsS390CCW(vm->def) && + virQEMUCapsGet(priv->qemuCaps, QEMU_CAPS_VIRTIO_CCW)) { + if (!virDomainDeviceAddressIsValid(&disk->info, + VIR_DOMAIN_DEVICE_ADDRESS_TYPE_CCW)) { + virReportError(VIR_ERR_OPERATION_FAILED, "%s", + _("device cannot be attached without a valid CCW address")); + goto cleanup; + } + } else if (virQEMUCapsGet(priv->qemuCaps, QEMU_CAPS_VIRTIO_S390)) { + if (!virDomainDeviceAddressIsValid(&disk->info, + VIR_DOMAIN_DEVICE_ADDRESS_TYPE_VIRTIO_S390)) { + virReportError(VIR_ERR_OPERATION_FAILED, "%s", + _("device cannot be attached without a valid S390 address")); + goto cleanup; + }
The whole purpose of qemuCheckCCWS390AddressSupport is to avoid adding more places where these type lines need to be added. John
+ } else if (!virDomainDeviceAddressIsValid(&disk->info, + VIR_DOMAIN_DEVICE_ADDRESS_TYPE_PCI)) { + virReportError(VIR_ERR_OPERATION_FAILED, "%s", + _("device cannot be attached without a valid PCI address")); + goto cleanup; + } }
for (i = 0; i < vm->def->ndisks; i++) {
----- Original Message -----
From: "John Ferlan" <jferlan@redhat.com> To: "Ruifeng Bian" <rbian@redhat.com>, libvir-list@redhat.com Sent: Wednesday, September 30, 2015 5:31:32 AM Subject: Re: [libvirt] [PATCH v4] qemu: Validate address type when attaching a disk device.
On 09/17/2015 03:35 AM, Ruifeng Bian wrote:
Bug fix for: https://bugzilla.redhat.com/show_bug.cgi?id=1257844
Attach-device can hotplug a virtio disk device with any address type now, it need to validate the address type before the attachment.
Attaching a disk device with --config option need to check address type. Otherwise, the domain cloudn't be started normally. This patch add a basic check for address type.
Detaching a disk device with --config also need to check disk options, add qemuCheckDiskConfig() method in qemuDomainDetachDeviceConfig().
Add virDomainDeviceAddressTypeIsValid method in domain_conf to check disk address type. Address type should match with disk driver, report error messages if any mismatch.
Signed-off-by: Ruifeng Bian <rbian@redhat.com> --- src/conf/domain_conf.c | 32 ++++++++++++++++++++++++++++++++ src/conf/domain_conf.h | 1 + src/libvirt_private.syms | 1 + src/qemu/qemu_command.c | 7 +++++++ src/qemu/qemu_driver.c | 2 ++ src/qemu/qemu_hotplug.c | 22 ++++++++++++++++++++++ 6 files changed, 65 insertions(+)
Just so this isn't left "hanging"... Michal has generated an alternate method to address the issue, see:
http://www.redhat.com/archives/libvir-list/2015-September/msg00909.html
I think there could be "synergies" between the two and left some thoughts in the other patch which use some of this code.
Some other comments below since they may be useful...
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 6df1618..f86760b 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -3200,6 +3200,38 @@ int virDomainDeviceAddressIsValid(virDomainDeviceInfoPtr info, return 0; }
+int virDomainDeviceAddressTypeIsValid(virDomainDiskDefPtr disk)
should be:
bool virDomainDeviceAddressTypeIsValid(virDomainDiskDefPtr disk)
+{ + if (disk->info.type) { + switch (disk->bus) { + case VIR_DOMAIN_DISK_BUS_IDE: + case VIR_DOMAIN_DISK_BUS_SCSI: + case VIR_DOMAIN_DISK_BUS_SATA: + case VIR_DOMAIN_DISK_BUS_FDC: + if (disk->info.type != VIR_DOMAIN_DEVICE_ADDRESS_TYPE_DRIVE) + return 1;
return false;
+ break; + case VIR_DOMAIN_DISK_BUS_USB: + if (disk->info.type != VIR_DOMAIN_DEVICE_ADDRESS_TYPE_USB) + return 1;
return false;
+ break; + case VIR_DOMAIN_DISK_BUS_VIRTIO: + if (disk->info.type != VIR_DOMAIN_DEVICE_ADDRESS_TYPE_CCW && + disk->info.type != VIR_DOMAIN_DEVICE_ADDRESS_TYPE_VIRTIO_S390 && + disk->info.type != VIR_DOMAIN_DEVICE_ADDRESS_TYPE_VIRTIO_MMIO && + disk->info.type != VIR_DOMAIN_DEVICE_ADDRESS_TYPE_PCI) + return 1;
return false;
+ break; + case VIR_DOMAIN_DISK_BUS_XEN: + case VIR_DOMAIN_DISK_BUS_UML: + case VIR_DOMAIN_DISK_BUS_SD: + /* no address type currently, return false if address provided */ + return 1;
return false;
+ } + } + return 0;
return true;
+} + virDomainDeviceInfoPtr virDomainDeviceGetInfo(virDomainDeviceDefPtr device) { diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h index f043554..337fe51 100644 --- a/src/conf/domain_conf.h +++ b/src/conf/domain_conf.h @@ -2543,6 +2543,7 @@ virDomainDeviceDefPtr virDomainDeviceDefCopy(virDomainDeviceDefPtr src, virDomainXMLOptionPtr xmlopt); int virDomainDeviceAddressIsValid(virDomainDeviceInfoPtr info, int type); +int virDomainDeviceAddressTypeIsValid(virDomainDiskDefPtr disk);
s/int/bool, probably unnecessary though
virDomainDeviceInfoPtr virDomainDeviceGetInfo(virDomainDeviceDefPtr device); int virDomainDeviceInfoCopy(virDomainDeviceInfoPtr dst, virDomainDeviceInfoPtr src); diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index 5b3da83..6cd5b9e 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -229,6 +229,7 @@ virDomainDefPostParse; virDomainDefSetMemoryInitial; virDomainDeleteConfig; virDomainDeviceAddressIsValid; +virDomainDeviceAddressTypeIsValid;
probably unnecessary
virDomainDeviceAddressTypeToString; virDomainDeviceDefCheckUnsupportedMemoryDevice; virDomainDeviceDefCopy; diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index 25f57f2..56ba08d 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -3606,6 +3606,13 @@ qemuCheckDiskConfig(virDomainDiskDefPtr disk) goto error; } } + + if (virDomainDeviceAddressTypeIsValid(disk)) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, + _("disk cannot have an address of type '%s'"), + virDomainDeviceAddressTypeToString(disk->info.type));
how about disk target '%s' using bus '%s' cannot have an address of type '%s', where target would be disk->dst and the bus would be decoded via virDomainDiskBusTypeToString(disk->bus), results in error message such as:
error: unsupported configuration: disk target 'vde' using bus 'virtio' cannot have an address of type 'drive'
Thanks, the message is clear now.
+ goto error; + }
But I'm not convinced the check should be put it here.
return 0; error: return -1; diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index fcf86b6..26c1502 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -8208,6 +8208,8 @@ qemuDomainDetachDeviceConfig(virDomainDefPtr vmdef, switch ((virDomainDeviceType) dev->type) { case VIR_DOMAIN_DEVICE_DISK: disk = dev->data.disk; + if (qemuCheckDiskConfig(disk) < 0) + return -1;
Why are we checking at Detach? Doesn't that already fail? Or is this just to get the "new" error message?
If we just want to detach a disk without attaching it before, it's necessary to check the configuration here.
if (!(det_disk = virDomainDiskRemoveByName(vmdef, disk->dst))) { virReportError(VIR_ERR_INVALID_ARG, _("no target device %s"), disk->dst); diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c index e84a78d..6156243 100644 --- a/src/qemu/qemu_hotplug.c +++ b/src/qemu/qemu_hotplug.c @@ -336,6 +336,28 @@ qemuDomainAttachVirtioDiskDevice(virConnectPtr conn, if (!qemuCheckCCWS390AddressSupport(vm->def, disk->info, priv->qemuCaps, disk->dst)) goto cleanup; + + if (qemuDomainMachineIsS390CCW(vm->def) && + virQEMUCapsGet(priv->qemuCaps, QEMU_CAPS_VIRTIO_CCW)) { + if (!virDomainDeviceAddressIsValid(&disk->info, + VIR_DOMAIN_DEVICE_ADDRESS_TYPE_CCW)) { + virReportError(VIR_ERR_OPERATION_FAILED, "%s", + _("device cannot be attached without a valid CCW address")); + goto cleanup; + } + } else if (virQEMUCapsGet(priv->qemuCaps, QEMU_CAPS_VIRTIO_S390)) { + if (!virDomainDeviceAddressIsValid(&disk->info, + VIR_DOMAIN_DEVICE_ADDRESS_TYPE_VIRTIO_S390)) { + virReportError(VIR_ERR_OPERATION_FAILED, "%s", + _("device cannot be attached without a valid S390 address")); + goto cleanup; + }
The whole purpose of qemuCheckCCWS390AddressSupport is to avoid adding more places where these type lines need to be added.
Yes, thanks.
John
+ } else if (!virDomainDeviceAddressIsValid(&disk->info, + VIR_DOMAIN_DEVICE_ADDRESS_TYPE_PCI)) { + virReportError(VIR_ERR_OPERATION_FAILED, "%s", + _("device cannot be attached without a valid PCI address")); + goto cleanup; + } }
for (i = 0; i < vm->def->ndisks; i++) {
participants (2)
-
John Ferlan -
Ruifeng Bian