-
Notifications
You must be signed in to change notification settings - Fork 13
[VDO-5752] [VDOSTORY-292] Add formatting logic #334
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: feature/add-vdoformat-into-kernel
Are you sure you want to change the base?
Changes from all commits
6174e13
b62997b
b9685f6
8af9dce
13f843d
37b2749
95ccd11
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,50 @@ | ||
| /* | ||
| * %COPYRIGHT% | ||
| * | ||
| * %LICENSE% | ||
| * | ||
| * $Id$ | ||
| */ | ||
|
|
||
| #include <linux/blkdev.h> | ||
| #include <linux/err.h> | ||
|
|
||
| #include "fileUtils.h" | ||
| #include "logger.h" | ||
| #include "memory-alloc.h" | ||
| #include "errors.h" | ||
|
|
||
| int blkdev_issue_zeroout(struct block_device *bdev, | ||
| sector_t sector, | ||
| sector_t nr_sects, | ||
| gfp_t gfp_mask __always_unused , | ||
| unsigned flags __always_unused) | ||
| { | ||
| int result; | ||
| off_t offset, length, file_size; | ||
| void *buffer; | ||
|
|
||
| offset = sector * SECTOR_SIZE; | ||
| length = nr_sects * SECTOR_SIZE; | ||
| file_size = bdev->size; | ||
|
|
||
| if ((offset + length) > file_size) { | ||
| return UDS_OUT_OF_RANGE; | ||
| } | ||
|
|
||
| result = vdo_allocate(length, u8, __func__, &buffer); | ||
| if (result != VDO_SUCCESS) | ||
| return result; | ||
|
|
||
| memset(buffer, 0, length); | ||
|
|
||
| result = write_buffer_at_offset(bdev->fd, offset, buffer, length); | ||
| if (result != UDS_SUCCESS) { | ||
| vdo_free(vdo_forget(buffer)); | ||
| return result; | ||
| } | ||
|
|
||
| result = logging_fsync(bdev->fd, "zero out"); | ||
| vdo_free(vdo_forget(buffer)); | ||
| return result; | ||
| } |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,15 @@ | ||
| /* | ||
| * User mode version of linux/uuid.h. | ||
| */ | ||
|
|
||
| #ifndef LINUX_UUID_H | ||
| #define LINUX_UUID_H | ||
|
|
||
| #include <uuid/uuid.h> | ||
|
|
||
| static inline void uuid_gen(uuid_t *uuid) | ||
| { | ||
| uuid_generate(*uuid); | ||
| } | ||
|
|
||
| #endif // LINUX_UUID_H |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -23,6 +23,16 @@ static const struct admin_state_code VDO_CODE_OPERATING = { | |
| .operating = true, | ||
| }; | ||
| const struct admin_state_code *VDO_ADMIN_STATE_OPERATING = &VDO_CODE_OPERATING; | ||
| static const struct admin_state_code VDO_CODE_KERNEL_FORMATTING = { | ||
| .name = "VDO_ADMIN_STATE_KERNEL_FORMATTING", | ||
| .operating = true, | ||
| .loading = true | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This edit seems unnecessary, and it looks like it puts this code out-of-step with the format of the other codes.
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The admin states are interesting. We could talk about them if that helps. I can also move them into a separate commit as you suggest. But in general, formatting is at the same stage as the pre-load stuff. which is the load phase, so as far as I understand from the code, it makes sense to make it .loading = true.
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I've renamed the new admin states and moved them to a separate commit. Hopefully it makes things clearer |
||
| }; | ||
| const struct admin_state_code *VDO_ADMIN_STATE_KERNEL_FORMATTING = &VDO_CODE_KERNEL_FORMATTING; | ||
| static const struct admin_state_code VDO_CODE_KERNEL_FORMATTED = { | ||
| .name = "VDO_ADMIN_STATE_KERNEL_FORMATTED", | ||
| }; | ||
| const struct admin_state_code *VDO_ADMIN_STATE_KERNEL_FORMATTED = &VDO_CODE_KERNEL_FORMATTED; | ||
| static const struct admin_state_code VDO_CODE_FORMATTING = { | ||
| .name = "VDO_ADMIN_STATE_FORMATTING", | ||
| .operating = true, | ||
|
|
@@ -174,8 +184,12 @@ static const struct admin_state_code *get_next_state(const struct admin_state *s | |
| if (operation == VDO_ADMIN_STATE_STOPPING) | ||
| return (code == VDO_ADMIN_STATE_NORMAL_OPERATION ? VDO_ADMIN_STATE_STOPPED : NULL); | ||
|
|
||
| if (operation == VDO_ADMIN_STATE_KERNEL_FORMATTING) | ||
| return (code == VDO_ADMIN_STATE_INITIALIZED ? VDO_ADMIN_STATE_KERNEL_FORMATTED : NULL); | ||
|
|
||
| if (operation == VDO_ADMIN_STATE_PRE_LOADING) | ||
| return (code == VDO_ADMIN_STATE_INITIALIZED ? VDO_ADMIN_STATE_PRE_LOADED : NULL); | ||
| return (((code == VDO_ADMIN_STATE_INITIALIZED) || | ||
| (code == VDO_ADMIN_STATE_KERNEL_FORMATTED)) ? VDO_ADMIN_STATE_PRE_LOADED : NULL); | ||
|
|
||
| if (operation == VDO_ADMIN_STATE_SUSPENDED_OPERATION) { | ||
| return (((code == VDO_ADMIN_STATE_SUSPENDED) || | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -65,6 +65,10 @@ | |
|
|
||
| #endif /* VDO_UPSTREAM */ | ||
| enum admin_phases { | ||
| FORMAT_PHASE_START, | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. It might be worthwhile to split this commit up (commit 3); it feels like it's doing two or three separable things. And the admin state changes are confusing enough it would be nice not to have them lumped in with other things.
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I guess I see it all as writing the layout to disk, but I will try to split the helper code into separate commits |
||
| FORMAT_PHASE_WRITE_SUPER, | ||
| FORMAT_PHASE_WRITE_GEOMETRY, | ||
| FORMAT_PHASE_END, | ||
| GROW_LOGICAL_PHASE_START, | ||
| GROW_LOGICAL_PHASE_GROW_BLOCK_MAP, | ||
| GROW_LOGICAL_PHASE_END, | ||
|
|
@@ -117,6 +121,10 @@ enum admin_phases { | |
| }; | ||
|
|
||
| static const char * const ADMIN_PHASE_NAMES[] = { | ||
| "FORMAT_PHASE_START", | ||
| "FORMAT_PHASE_WRITE_SUPER", | ||
| "FORMAT_PHASE_WRITE_GEOMETRY", | ||
| "FORMAT_PHASE_END", | ||
| "GROW_LOGICAL_PHASE_START", | ||
| "GROW_LOGICAL_PHASE_GROW_BLOCK_MAP", | ||
| "GROW_LOGICAL_PHASE_END", | ||
|
|
@@ -1702,6 +1710,48 @@ static int __must_check decode_vdo(struct vdo *vdo) | |
| return vdo_make_hash_zones(vdo, &vdo->hash_zones); | ||
| } | ||
|
|
||
| /** | ||
| * format_callback() - Callback to initiate a format, registered in vdo_initialize(). | ||
| * @completion: The admin completion. | ||
| */ | ||
| static void format_callback(struct vdo_completion *completion) | ||
| { | ||
| struct vdo *vdo = completion->vdo; | ||
| int result; | ||
|
|
||
| assert_admin_phase_thread(vdo, __func__); | ||
|
|
||
| switch (advance_phase(vdo)) { | ||
| case FORMAT_PHASE_START: | ||
| result = vdo_start_operation(&vdo->admin.state, VDO_ADMIN_STATE_KERNEL_FORMATTING); | ||
| if (result != VDO_SUCCESS) { | ||
| vdo_continue_completion(completion, result); | ||
| return; | ||
| } | ||
|
|
||
| vdo_continue_completion(completion, vdo_clear_layout(vdo)); | ||
| return; | ||
|
|
||
| case FORMAT_PHASE_WRITE_SUPER: | ||
| vdo_save_super_block(vdo, completion); | ||
| return; | ||
|
|
||
| case FORMAT_PHASE_WRITE_GEOMETRY: | ||
| vdo_save_geometry_block(vdo, completion); | ||
| return; | ||
|
|
||
| case FORMAT_PHASE_END: | ||
| /* Clean up the states layout to prevent corruption during pre-load decode */ | ||
| vdo_uninitialize_layout(&vdo->states.layout); | ||
| break; | ||
|
|
||
| default: | ||
| vdo_set_completion_result(completion, UDS_BAD_STATE); | ||
| } | ||
|
|
||
| finish_operation_callback(completion); | ||
| } | ||
|
|
||
| /** | ||
| * pre_load_callback() - Callback to initiate a pre-load, registered in vdo_initialize(). | ||
| * @completion: The admin completion. | ||
|
|
@@ -1805,6 +1855,20 @@ static int vdo_initialize(struct dm_target *ti, unsigned int instance, | |
| return result; | ||
| } | ||
|
|
||
| if (vdo_get_state(vdo) == VDO_NEW) { | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I'm not sure cheking VDO_NEW here is what you want. I think that if a VDO is started and then stopped without any writes, it stays in the VDO_NEW state. But in that case, we'd re-save and rewrite everything... even though we actually just read it off disk to begin with. Do we need a new flag or a new state here?
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I thought about this for some time. VDO_NEW was basically set previously only in the user mode code when we initialized the component states prior to writing out to disk using vdoformat. In other words, in my mind, it was vdoformat telling the kernel code that I formatted the device for the first time. I was trying to mimic that as well with the formatting in the kernel code. So its either: 1. vdoformat runs and set the state to new and saves it to disk then the kernel loads the super block in pre-load and now sees that state. or 2. vdo_format is called by the kernel which initializes the component states, sets the state to new and saves it to disk then the kernel loads the super block in pre-load and now sees that state. |
||
| result = perform_admin_operation(vdo, FORMAT_PHASE_START, format_callback, | ||
| finish_operation_callback, "format"); | ||
| if (result != VDO_SUCCESS) { | ||
| ti->error = ((result == VDO_INVALID_ADMIN_STATE) ? | ||
| "Format is only valid immediately after initialization" : | ||
| "Cannot format device"); | ||
| vdo_log_error("Could not format VDO device. (VDO error %d, message %s)", | ||
| result, ti->error); | ||
| vdo_destroy(vdo); | ||
| return result; | ||
| } | ||
| } | ||
|
|
||
| result = perform_admin_operation(vdo, PRE_LOAD_PHASE_START, pre_load_callback, | ||
| finish_operation_callback, "pre-load"); | ||
| if (result != VDO_SUCCESS) { | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why did you move this typedef? I mean the move seems fine but it's three files that seem like they didn't actually have to change?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
the new function is in uds. I believe you wanted it there. the original typedef was in vdo/fake, so I moved it to uds as well